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;