From 1d3e930fbd57788ed60494a64835127ff97dbd53 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Wed, 28 Jun 2023 00:56:58 +0300 Subject: [PATCH] Don't allow previewing `shared` history rooms (#239) Only `world_readable` can be considered as opting into having history publicly on the web. Anything else must not be archived until there's a dedicated state event for opting into archiving. --- server/lib/matrix-utils/fetch-room-data.js | 12 ------ server/routes/room-directory-routes.js | 4 +- server/routes/room-routes.js | 12 +++--- test/e2e-tests.js | 45 +++++++++++++++++++--- 4 files changed, 47 insertions(+), 26 deletions(-) diff --git a/server/lib/matrix-utils/fetch-room-data.js b/server/lib/matrix-utils/fetch-room-data.js index 91c702e..e1ea483 100644 --- a/server/lib/matrix-utils/fetch-room-data.js +++ b/server/lib/matrix-utils/fetch-room-data.js @@ -155,7 +155,6 @@ const fetchRoomData = traceFunction(async function ( stateCanonicalAliasResDataOutcome, stateAvatarResDataOutcome, stateHistoryVisibilityResDataOutcome, - stateJoinRulesResDataOutcome, predecessorInfoOutcome, successorInfoOutcome, ] = await Promise.allSettled([ @@ -182,10 +181,6 @@ const fetchRoomData = traceFunction(async function ( abortSignal, } ), - fetchEndpointAsJson(getStateEndpointForRoomIdAndEventType(roomId, 'm.room.join_rules'), { - accessToken: matrixAccessToken, - abortSignal, - }), fetchPredecessorInfo(matrixAccessToken, roomId, { abortSignal }), fetchSuccessorInfo(matrixAccessToken, roomId, { abortSignal }), ]); @@ -220,12 +215,6 @@ const fetchRoomData = traceFunction(async function ( historyVisibility = data?.content?.history_visibility; } - let joinRule; - if (stateJoinRulesResDataOutcome.reason === undefined) { - const { data } = stateJoinRulesResDataOutcome.value; - joinRule = data?.content?.join_rule; - } - let roomCreationTs; let predecessorRoomId; let predecessorLastKnownEventId; @@ -251,7 +240,6 @@ const fetchRoomData = traceFunction(async function ( canonicalAlias, avatarUrl, historyVisibility, - joinRule, roomCreationTs, predecessorRoomId, predecessorLastKnownEventId, diff --git a/server/routes/room-directory-routes.js b/server/routes/room-directory-routes.js index 7b88671..ad87681 100644 --- a/server/routes/room-directory-routes.js +++ b/server/routes/room-directory-routes.js @@ -22,7 +22,6 @@ const matrixServerName = config.get('matrixServerName'); assert(matrixServerName); const matrixAccessToken = config.get('matrixAccessToken'); assert(matrixAccessToken); -const stopSearchEngineIndexing = config.get('stopSearchEngineIndexing'); const router = express.Router({ caseSensitive: true, @@ -71,7 +70,8 @@ router.get( } // We index the room directory unless the config says we shouldn't index anything - const shouldIndex = !stopSearchEngineIndexing; + const stopSearchEngineIndexingFromConfig = config.get('stopSearchEngineIndexing'); + const shouldIndex = !stopSearchEngineIndexingFromConfig; const pageOptions = { title: `Matrix Public Archive`, diff --git a/server/routes/room-routes.js b/server/routes/room-routes.js index c58dd7e..7417ac9 100644 --- a/server/routes/room-routes.js +++ b/server/routes/room-routes.js @@ -57,7 +57,6 @@ const matrixServerUrl = config.get('matrixServerUrl'); assert(matrixServerUrl); const matrixAccessToken = config.get('matrixAccessToken'); assert(matrixAccessToken); -const stopSearchEngineIndexing = config.get('stopSearchEngineIndexing'); const matrixPublicArchiveURLCreator = new MatrixPublicArchiveURLCreator(basePath); @@ -828,15 +827,13 @@ router.get( }), ]); - // Only `world_readable` or `shared` rooms that are `public` are viewable in the archive - const allowedToViewRoom = - roomData.historyVisibility === 'world_readable' || - (roomData.historyVisibility === 'shared' && roomData.joinRule === 'public'); + // Only `world_readable` rooms are viewable in the archive + const allowedToViewRoom = roomData.historyVisibility === 'world_readable'; if (!allowedToViewRoom) { throw new StatusError( 403, - `Only \`world_readable\` or \`shared\` rooms that are \`public\` can be viewed in the archive. ${roomData.id} has m.room.history_visiblity=${roomData.historyVisibility} m.room.join_rules=${roomData.joinRule}` + `Only \`world_readable\` rooms can be viewed in the archive. ${roomData.id} has m.room.history_visiblity=${roomData.historyVisibility}` ); } @@ -891,7 +888,8 @@ router.get( // Default to no indexing (safe default) let shouldIndex = false; - if (stopSearchEngineIndexing) { + const stopSearchEngineIndexingFromConfig = config.get('stopSearchEngineIndexing'); + if (stopSearchEngineIndexingFromConfig) { shouldIndex = false; } else { // Otherwise we only allow search engines to index `world_readable` rooms diff --git a/test/e2e-tests.js b/test/e2e-tests.js index f7b4bc2..b95425c 100644 --- a/test/e2e-tests.js +++ b/test/e2e-tests.js @@ -2688,15 +2688,50 @@ describe('matrix-public-archive', () => { assert.strictEqual(dom.document.querySelector(`meta[name="robots"]`), null); }); - it('search engines not allowed to index `public` room', async () => { + it('search engines not allowed to access public room with non-`world_readable` history visibility', async () => { const client = await getTestClientForHs(testMatrixServerUrl1); const roomId = await createTestRoom(client, { - // The default options for the test rooms adds a - // `m.room.history_visiblity` state event so we override that here so - // it's only a public room. - initial_state: [], + // Set as `shared` since it's the next most permissive history visibility + // after `world_readable` but still not allowed to be accesible in the + // archive. + initial_state: [ + { + type: 'm.room.history_visibility', + state_key: '', + content: { + history_visibility: 'shared', + }, + }, + ], }); + try { + archiveUrl = matrixPublicArchiveURLCreator.archiveUrlForRoom(roomId); + await fetchEndpointAsText(archiveUrl); + assert.fail( + new TestError( + 'We expect the request to fail with a 403 since the archive should not be able to view a non-world_readable room but it succeeded' + ) + ); + } catch (err) { + if (err instanceof TestError) { + throw err; + } + assert.strictEqual( + err.response.status, + 403, + `Expected err.response.status=${err?.response?.status} to be 403 but error was: ${err.stack}` + ); + } + }); + + it('Configuring `stopSearchEngineIndexing` will stop search engine indexing', async () => { + // Disable search engine indexing across the entire instance + config.set('stopSearchEngineIndexing', true); + + const client = await getTestClientForHs(testMatrixServerUrl1); + const roomId = await createTestRoom(client); + archiveUrl = matrixPublicArchiveURLCreator.archiveUrlForRoom(roomId); const { data: archivePageHtml } = await fetchEndpointAsText(archiveUrl);