Skip to content

Commit

Permalink
Don't allow previewing shared history rooms (#239)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tulir authored Jun 27, 2023
1 parent e480085 commit 1d3e930
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 26 deletions.
12 changes: 0 additions & 12 deletions server/lib/matrix-utils/fetch-room-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ const fetchRoomData = traceFunction(async function (
stateCanonicalAliasResDataOutcome,
stateAvatarResDataOutcome,
stateHistoryVisibilityResDataOutcome,
stateJoinRulesResDataOutcome,
predecessorInfoOutcome,
successorInfoOutcome,
] = await Promise.allSettled([
Expand All @@ -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 }),
]);
Expand Down Expand Up @@ -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;
Expand All @@ -251,7 +240,6 @@ const fetchRoomData = traceFunction(async function (
canonicalAlias,
avatarUrl,
historyVisibility,
joinRule,
roomCreationTs,
predecessorRoomId,
predecessorLastKnownEventId,
Expand Down
4 changes: 2 additions & 2 deletions server/routes/room-directory-routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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`,
Expand Down
12 changes: 5 additions & 7 deletions server/routes/room-routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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}`
);
}

Expand Down Expand Up @@ -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
Expand Down
45 changes: 40 additions & 5 deletions test/e2e-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 1d3e930

Please sign in to comment.