From 0fb3df54b20cd47311a84c055604a0749980ab98 Mon Sep 17 00:00:00 2001 From: Travis Rich Date: Wed, 9 Sep 2026 14:35:19 -0400 Subject: [PATCH] fix: let sharing links into a CMS-mode community CMS mode was admitting members only, so a Sharing/View URL like `/pub//draft?access=` returned "We can't find that community" to the very people it is for. Authors could not reach their own proofs without first being added as members of the community, which puts an account-provisioning step in front of every review. The cause was ordering, not policy: the gate ran before `getScope`, which is the only place an `?access=` hash is resolved, so the hash was never consulted. The community 404'd first. That also explains why changing the discussion permissions had no effect -- this is a community-existence gate, not a permission one. So the CMS gate now runs after `getScope` and keys on `activePermissions.canView`, which is precisely "insider": a member of the community, collection or pub, a superadmin, or the holder of an access hash matching something in this URL. Public permissions do not raise it, and neither does a pub being released (a released pub is visible via `releases.length`, a separate path), so the community stays invisible to the public. One pub's hash does not unlock another, and the community home -- which no hash can match -- stays gated. The spam gate keeps its old position and its members-only rule. A spam-flagged community has no way to opt a visitor back in, so it needs nothing from the scope; only CMS mode does. Two consequences worth noting: - Requests that are turned away now run `getScope` before 404ing. It already ran for every request that got through. - On a custom-domain community the domain redirect now precedes the gate, so a subdomain request redirects and is gated at the destination. The banner in App.tsx keyed on merely being logged in, which would now show "only Members can see this page" to an author who arrived by link; it keys on community membership instead. Tests: `isCmsGated` covers the gate decision. A DB-backed scopeGet test pins the load-bearing assumption -- that `canView` is false for anonymous and logged-in non-member visitors to a released pub carrying the most permissive public permissions, false for a wrong hash and for another pub's hash, and true for a member and for view/comment hash holders. --- client/containers/App/App.tsx | 5 +- server/utils/initData.ts | 39 +++++- .../__tests__/scopeGetCanView.test.ts | 127 ++++++++++++++++++ utils/__tests__/cms.test.ts | 30 ++++- utils/cms.ts | 20 +++ 5 files changed, 212 insertions(+), 9 deletions(-) create mode 100644 server/utils/queryHelpers/__tests__/scopeGetCanView.test.ts diff --git a/client/containers/App/App.tsx b/client/containers/App/App.tsx index 8c88d669ec..bb6678e90e 100644 --- a/client/containers/App/App.tsx +++ b/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)); diff --git a/server/utils/initData.ts b/server/utils/initData.ts index 1f495a8b75..9296124494 100644 --- a/server/utils/initData.ts +++ b/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, diff --git a/server/utils/queryHelpers/__tests__/scopeGetCanView.test.ts b/server/utils/queryHelpers/__tests__/scopeGetCanView.test.ts new file mode 100644 index 0000000000..7a0990fa78 --- /dev/null +++ b/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); + }); +}); diff --git a/utils/__tests__/cms.test.ts b/utils/__tests__/cms.test.ts index 245744feda..a19ec7185a 100644 --- a/utils/__tests__/cms.test.ts +++ b/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); + }); +}); diff --git a/utils/cms.ts b/utils/cms.ts index 43735dda64..ff8fe8d1fb 100644 --- a/utils/cms.ts +++ b/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;