From d1a411744177bc9c652db4d6e5261e19bfaf6120 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Tue, 30 May 2023 17:34:40 +0300 Subject: [PATCH 1/3] Don't allow previewing `shared` history rooms 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/routes/room-routes.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/server/routes/room-routes.js b/server/routes/room-routes.js index 2044993f..da167599 100644 --- a/server/routes/room-routes.js +++ b/server/routes/room-routes.js @@ -828,15 +828,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} m.room.join_rules=${roomData.joinRule}` ); } From 53de3b64ad739ae7fa261914df7179adc6a4f151 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Jun 2023 16:39:48 -0500 Subject: [PATCH 2/3] Add tests to ensure we don't show shared rooms in the archive Tests for https://github.com/matrix-org/matrix-public-archive/pull/239 --- server/routes/room-directory-routes.js | 4 +-- server/routes/room-routes.js | 4 +-- test/e2e-tests.js | 45 +++++++++++++++++++++++--- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/server/routes/room-directory-routes.js b/server/routes/room-directory-routes.js index 468d8d60..2d1e6ed8 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 da167599..0fdb4060 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); @@ -889,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 9f21fdfd..549c7319 100644 --- a/test/e2e-tests.js +++ b/test/e2e-tests.js @@ -2686,15 +2686,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); From f750b4fcc2ff671cbe2c04c1f00fe59e77bd9529 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 27 Jun 2023 16:42:03 -0500 Subject: [PATCH 3/3] We no longer need to factor in and fetch the join rules --- server/lib/matrix-utils/fetch-room-data.js | 12 ------------ server/routes/room-routes.js | 2 +- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/server/lib/matrix-utils/fetch-room-data.js b/server/lib/matrix-utils/fetch-room-data.js index 91c702e6..e1ea483d 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-routes.js b/server/routes/room-routes.js index 0fdb4060..6b6e17df 100644 --- a/server/routes/room-routes.js +++ b/server/routes/room-routes.js @@ -833,7 +833,7 @@ router.get( if (!allowedToViewRoom) { throw new StatusError( 403, - `Only \`world_readable\` rooms 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}` ); }