fix: let sharing links into a CMS-mode community

client/containers/App/App.tsx

@@ -79,9 +79,12 @@ const App = (props: Props) => {
 	// In CMS mode, public visitors get a not-found page; warn the members who
 	// can see the content that it isn't publicly visible. Only shown on public
 	// content views (pages, collections, and pub releases — not drafts).
+	// Keyed on community membership rather than merely being logged in: someone
+	// who got here through a sharing link can see the page without being a
+	// member, and telling them only members can see it is just confusing.
 	const showCmsModeBanner =
 		communityData.cmsMode &&
-		!!loginData.id &&
+		scopeData.activePermissions.canViewCommunity &&
 		(chunkName === 'Page' ||
 			chunkName === 'Collection' ||
 			(chunkName === 'Pub' && !!viewData.pubData?.isRelease));

server/utils/initData.ts

@@ -8,7 +8,7 @@ import { isUserMemberOfScope } from 'server/member/queries';
 import { UserNotification } from 'server/models';
 import { isUserSuperAdmin } from 'server/user/queries';
 import { getDismissedUserDismissables } from 'server/userDismissable/queries';
-import { isAuthBypassPath, isCmsGateBypassPath } from 'utils/cms';
+import { isAuthBypassPath, isCmsGated } from 'utils/cms';
 import { getAppCommit, isDuqDuq, isProd, isQubQub, shouldForceBasePubPub } from 'utils/environment';
 
 import { PubPubError } from './errors';
@@ -132,12 +132,15 @@ export const getInitialData = async (
 			: { domain: hostname };
 	const communityData = await getCommunity(locationData, whereQuery);
 
+	/* A spam-flagged community is hidden from everyone but its own members, with
+	   no way to opt a visitor back in -- so this gate needs nothing from the
+	   scope and stays here, ahead of the work below. The CMS-mode gate is the
+	   opposite (sharing links must keep working) and runs after getScope. */
 	const isSpamGated =
 		communityData.spamTag &&
 		communityData.spamTag.status !== 'confirmed-not-spam' &&
 		!isAuthBypassPath(req.path);
-	const isCmsGated = communityData.cmsMode && !isCmsGateBypassPath(req.path);
-	if (isSpamGated || isCmsGated) {
+	if (isSpamGated) {
 		const [isMemberOfCommunity, isSuperadmin] = await Promise.all([
 			isUserMemberOfScope({
 				userId: loginData.id,
@@ -146,10 +149,7 @@ export const getInitialData = async (
 			isUserSuperAdmin({ userId: loginData.id }),
 		]);
 		if (!isMemberOfCommunity && !isSuperadmin) {
-			if (isSpamGated) {
-				throw new PubPubError.CommunityIsSpamError();
-			}
-			throw new PubPubError.CommunityIsPrivateError();
+			throw new PubPubError.CommunityIsSpamError();
 		}
 	}
 
@@ -186,6 +186,31 @@ export const getInitialData = async (
 			await getDismissedUserDismissables(user.id),
 		]);
 
+	/**
+	 * A CMS-mode community is invisible to the public, but a sharing link has to
+	 * keep working: authors review their proofs through one, and requiring
+	 * membership puts an account-provisioning step in front of every proof.
+	 *
+	 * `canView` is exactly the predicate we want. getScope raises it only for
+	 * members of the community/collection/pub, superadmins, and holders of an
+	 * access hash matching something in *this* URL. Neither public permissions
+	 * nor a pub being released raise it -- released pubs are visible because
+	 * `releases.length` is non-zero, a separate path -- so the community stays
+	 * hidden from the public while `?access=` links resolve normally.
+	 *
+	 * This has to run after getScope, which is where the access hash is
+	 * resolved; the spam gate above needs no scope and stays ahead of it.
+	 */
+	if (
+		isCmsGated({
+			cmsMode: communityData.cmsMode,
+			path: req.path,
+			canView: scopeData.activePermissions.canView,
+		})
+	) {
+		throw new PubPubError.CommunityIsPrivateError();
+	}
+
 	const cleanedCommunityData = sanitizeCommunity(
 		communityData,
 		locationData,

server/utils/queryHelpers/__tests__/scopeGetCanView.test.ts

@@ -0,0 +1,127 @@
+import { getScope } from 'server/utils/queryHelpers';
+import { modelize, setup, teardown } from 'stubstub';
+
+/**
+ * `canView` is the predicate the CMS-mode gate wants to key on, so pin down
+ * exactly what raises it: membership, a matching access hash, or superadmin --
+ * and NOT a pub being released or carrying public permissions.
+ */
+const models = modelize`
+	Community community {
+		Member {
+			permissions: "view"
+			User communityViewer {}
+		}
+		Pub releasedPub {
+			slug: "released-pub"
+			viewHash: "view-hash-abc"
+			commentHash: "comment-hash-abc"
+			Release {}
+		}
+		Pub draftPub {
+			slug: "draft-pub"
+			viewHash: "draft-view-hash"
+		}
+	}
+	User randomLoggedInUser {}
+`;
+
+setup(beforeAll, async () => {
+	await models.resolve();
+	const { releasedPub, community } = models;
+	// The most permissive public permissions PubPub can express, at both the
+	// pub and community level.
+	const { PublicPermissions } = await import('server/models');
+	await PublicPermissions.create({
+		pubId: releasedPub.id,
+		canCreateReviews: true,
+		canCreateDiscussions: true,
+		discussionCreationAccess: 'public',
+		canViewDraft: true,
+		canEditDraft: true,
+	});
+	await PublicPermissions.create({
+		communityId: community.id,
+		canCreateReviews: true,
+		canCreateDiscussions: true,
+		discussionCreationAccess: 'public',
+		canViewDraft: true,
+		canEditDraft: true,
+	});
+});
+
+teardown(afterAll);
+
+const scopeFor = (opts: { pubSlug?: string; accessHash?: string; loginId?: string | null }) =>
+	getScope({
+		communityId: models.community.id,
+		pubSlug: opts.pubSlug,
+		accessHash: opts.accessHash ?? null,
+		loginId: opts.loginId ?? null,
+	});
+
+describe('canView is not granted by release or public permissions', () => {
+	it('is false for an anonymous visitor to a RELEASED pub with public permissions', async () => {
+		const { activePermissions } = await scopeFor({ pubSlug: 'released-pub' });
+		expect(activePermissions.canView).toEqual(false);
+		expect(activePermissions.activePermission).toEqual(null);
+		// ...even though the public permissions really did apply:
+		expect(activePermissions.canViewDraft).toEqual(true);
+		expect(activePermissions.canCreateDiscussions).toEqual(true);
+	});
+
+	it('is false for a logged-in non-member on the same released pub', async () => {
+		const { activePermissions } = await scopeFor({
+			pubSlug: 'released-pub',
+			loginId: models.randomLoggedInUser.id,
+		});
+		expect(activePermissions.canView).toEqual(false);
+	});
+
+	it('is false for an anonymous visitor to the community root', async () => {
+		const { activePermissions } = await scopeFor({});
+		expect(activePermissions.canView).toEqual(false);
+	});
+
+	it('is false when the access hash is wrong', async () => {
+		const { activePermissions } = await scopeFor({
+			pubSlug: 'released-pub',
+			accessHash: 'not-the-right-hash',
+		});
+		expect(activePermissions.canView).toEqual(false);
+	});
+});
+
+describe('canView IS granted by membership or a matching access hash', () => {
+	it('is true for a community member with view permissions', async () => {
+		const { activePermissions } = await scopeFor({
+			pubSlug: 'released-pub',
+			loginId: models.communityViewer.id,
+		});
+		expect(activePermissions.canView).toEqual(true);
+	});
+
+	it('is true for an anonymous holder of the pub viewHash', async () => {
+		const { activePermissions } = await scopeFor({
+			pubSlug: 'released-pub',
+			accessHash: 'view-hash-abc',
+		});
+		expect(activePermissions.canView).toEqual(true);
+	});
+
+	it('is true for an anonymous holder of the pub commentHash', async () => {
+		const { activePermissions } = await scopeFor({
+			pubSlug: 'released-pub',
+			accessHash: 'comment-hash-abc',
+		});
+		expect(activePermissions.canView).toEqual(true);
+	});
+
+	it("does not let one pub's hash unlock a different pub", async () => {
+		const { activePermissions } = await scopeFor({
+			pubSlug: 'draft-pub',
+			accessHash: 'view-hash-abc',
+		});
+		expect(activePermissions.canView).toEqual(false);
+	});
+});

utils/__tests__/cms.test.ts

@@ -5,7 +5,7 @@ import {
 	canonicalCommunityUrl,
 	canonicalPubUrl,
 } from 'utils/canonicalUrls';
-import { isAuthBypassPath, isCmsGateBypassPath } from 'utils/cms';
+import { isAuthBypassPath, isCmsGateBypassPath, isCmsGated } from 'utils/cms';
 
 const plainCommunity = { subdomain: 'demo', domain: null } as any;
 const cmsCommunity = {
@@ -90,3 +90,31 @@ describe('isCmsGateBypassPath', () => {
 		expect(isCmsGateBypassPath('/loginish')).toBe(false);
 	});
 });
+
+describe('isCmsGated', () => {
+	const path = '/pub/my-pub/draft';
+
+	it('turns away a public visitor to a CMS-mode community', () => {
+		expect(isCmsGated({ cmsMode: true, path, canView: false })).toBe(true);
+	});
+
+	it('lets an insider through — a member, superadmin, or access-link holder', () => {
+		expect(isCmsGated({ cmsMode: true, path, canView: true })).toBe(false);
+	});
+
+	it('does nothing at all when the community is not in CMS mode', () => {
+		expect(isCmsGated({ cmsMode: false, path, canView: false })).toBe(false);
+		expect(isCmsGated({ cmsMode: null, path, canView: false })).toBe(false);
+		expect(isCmsGated({ cmsMode: undefined, path, canView: false })).toBe(false);
+	});
+
+	it('never gates a bypass path, so members can still sign in and crawlers can read the rules', () => {
+		expect(isCmsGated({ cmsMode: true, path: '/login', canView: false })).toBe(false);
+		expect(isCmsGated({ cmsMode: true, path: '/dash/settings', canView: false })).toBe(false);
+		expect(isCmsGated({ cmsMode: true, path: '/robots.txt', canView: false })).toBe(false);
+	});
+
+	it('gates the community home, which no access hash can match', () => {
+		expect(isCmsGated({ cmsMode: true, path: '/', canView: false })).toBe(true);
+	});
+});

utils/cms.ts

@@ -29,3 +29,23 @@ export const isCmsGateBypassPath = (path: string) =>
 	/^\/sitemap[^/]*\.xml$/.test(path) ||
 	isAuthBypassPath(path) ||
 	matchesPrefix(path, ['/dash']);
+
+/**
+ * Should this request be turned away from a CMS-mode community?
+ *
+ * `canView` comes from `scopeData.activePermissions` and is the definition of
+ * an insider: getScope raises it only for members of the community, collection
+ * or pub, for superadmins, and for holders of an access hash matching
+ * something in the request's own URL. Public permissions do not raise it, and
+ * neither does a pub being released, so the community stays invisible to the
+ * public while sharing links keep working.
+ */
+export const isCmsGated = ({
+	cmsMode,
+	path,
+	canView,
+}: {
+	cmsMode: boolean | null | undefined;
+	path: string;
+	canView: boolean;
+}) => Boolean(cmsMode) && !isCmsGateBypassPath(path) && !canView;