From a20f367d8bc2ed47fb75efd24be5dff7d3a903c4 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 Dec 2018 19:51:01 +0000 Subject: [PATCH 01/26] Resolve homeservers' names through DNS if possible --- services/matrix/matrix.service.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index a6199a35ec697..23335e2c4364b 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -1,5 +1,7 @@ 'use strict' +const dns = require('dns') +const util = require('util') const Joi = require('joi') const BaseJsonService = require('../base-json') @@ -39,7 +41,14 @@ const documentation = `

` +const srvPrefix = '_matrix._tcp.' +const resolve = util.promisify(dns.resolveSrv) + module.exports = class Matrix extends BaseJsonService { + async lookupMatrixHomeserver(host) { + return resolve(srvPrefix + host) + } + async registerAccount({ host, guest }) { return this._requestJson({ url: `https://${host}/_matrix/client/r0/register`, @@ -65,6 +74,16 @@ module.exports = class Matrix extends BaseJsonService { } async fetch({ host, roomId }) { + try { + const addrs = await this.lookupMatrixHomeserver(host) + if (addrs.length) { + host = addrs[0].name + } + } catch (e) { + if (e.code !== 'ENOTFOUND') { + throw e + } + } let auth try { auth = await this.registerAccount({ host, guest: true }) From d3f411016dea302d4db0ad6fd67e86ab6936da84 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 Dec 2018 20:14:23 +0000 Subject: [PATCH 02/26] Add a connectivity check before trying to register Some Matrix homeserver only use the address in the SRV record for federation, which uses its own set of routes (and can sometimes use self-signed certificates, or certs using the server's name rather than the address in their SRV record). Therefore, we check if the client can contact the client APIs beforehand, and fallback to the server's name in case of an error. --- services/matrix/matrix.service.js | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 23335e2c4364b..6fdecb7e20c3c 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -9,6 +9,12 @@ const matrixRegisterSchema = Joi.object({ access_token: Joi.string().required(), }).required() +const matrixClientVersionsSchema = Joi.object({ + versions: Joi.array() + .items(Joi.string().required()) + .required(), +}).required() + const matrixStateSchema = Joi.array() .items( Joi.object({ @@ -49,6 +55,13 @@ module.exports = class Matrix extends BaseJsonService { return resolve(srvPrefix + host) } + async checkMatrixHomeserverClientAPI(host) { + return this._requestJson({ + url: `https://${host}/_matrix/client/versions`, + schema: matrixClientVersionsSchema, + }) + } + async registerAccount({ host, guest }) { return this._requestJson({ url: `https://${host}/_matrix/client/r0/register`, @@ -77,9 +90,20 @@ module.exports = class Matrix extends BaseJsonService { try { const addrs = await this.lookupMatrixHomeserver(host) if (addrs.length) { - host = addrs[0].name + // The address we are given may be only to use for federation. Therefore + // we check if we can painlessly reach the client APIs at this address, + // and if not we don't do anything, and ignore the error, since host + // already holds the right value, and we expect this check to fail in + // some cases. + try { + await this.checkMatrixHomeserverClientAPI(addrs[0].name) + host = addrs[0].name + } catch (e) {} } } catch (e) { + // If the error is ENOTFOUND, it means that there is no SRV record for + // this server, and that we need to fall back on the value host already + // holds. if (e.code !== 'ENOTFOUND') { throw e } From d02e309e8f4ac2932e637b2d31825d6c9baaa6f2 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 Dec 2018 20:20:24 +0000 Subject: [PATCH 03/26] Comply with the overall coding style --- services/matrix/matrix.service.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 6fdecb7e20c3c..fc60d37d2809a 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -51,11 +51,11 @@ const srvPrefix = '_matrix._tcp.' const resolve = util.promisify(dns.resolveSrv) module.exports = class Matrix extends BaseJsonService { - async lookupMatrixHomeserver(host) { + async lookupMatrixHomeserver({ host }) { return resolve(srvPrefix + host) } - async checkMatrixHomeserverClientAPI(host) { + async checkMatrixHomeserverClientAPI({ host }) { return this._requestJson({ url: `https://${host}/_matrix/client/versions`, schema: matrixClientVersionsSchema, @@ -88,7 +88,7 @@ module.exports = class Matrix extends BaseJsonService { async fetch({ host, roomId }) { try { - const addrs = await this.lookupMatrixHomeserver(host) + const addrs = await this.lookupMatrixHomeserver({ host }) if (addrs.length) { // The address we are given may be only to use for federation. Therefore // we check if we can painlessly reach the client APIs at this address, @@ -96,7 +96,7 @@ module.exports = class Matrix extends BaseJsonService { // already holds the right value, and we expect this check to fail in // some cases. try { - await this.checkMatrixHomeserverClientAPI(addrs[0].name) + await this.checkMatrixHomeserverClientAPI({ host: addrs[0].name }) host = addrs[0].name } catch (e) {} } From ffc32829bcdcc48ca5dc5d1425a0a51512bc7294 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 Dec 2018 21:24:12 +0000 Subject: [PATCH 04/26] Bonus: Use room aliases (with correct syntax) instead of room IDs --- services/matrix/matrix.service.js | 89 +++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 28 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index fc60d37d2809a..732c1a4ecee00 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -4,6 +4,7 @@ const dns = require('dns') const util = require('util') const Joi = require('joi') const BaseJsonService = require('../base-json') +const errors = require('../errors') const matrixRegisterSchema = Joi.object({ access_token: Joi.string().required(), @@ -15,6 +16,10 @@ const matrixClientVersionsSchema = Joi.object({ .required(), }).required() +const matrixAliasLookupSchema = Joi.object({ + room_id: Joi.string().required(), +}) + const matrixStateSchema = Joi.array() .items( Joi.object({ @@ -86,27 +91,57 @@ module.exports = class Matrix extends BaseJsonService { }) } - async fetch({ host, roomId }) { - try { - const addrs = await this.lookupMatrixHomeserver({ host }) - if (addrs.length) { - // The address we are given may be only to use for federation. Therefore - // we check if we can painlessly reach the client APIs at this address, - // and if not we don't do anything, and ignore the error, since host - // already holds the right value, and we expect this check to fail in - // some cases. - try { - await this.checkMatrixHomeserverClientAPI({ host: addrs[0].name }) - host = addrs[0].name - } catch (e) {} - } - } catch (e) { - // If the error is ENOTFOUND, it means that there is no SRV record for - // this server, and that we need to fall back on the value host already - // holds. - if (e.code !== 'ENOTFOUND') { - throw e + async lookupRoomAlias({ host, roomAlias, auth }) { + return this._requestJson({ + url: `https://${host}/_matrix/client/r0/directory/room/%23${roomAlias}`, + schema: matrixAliasLookupSchema, + options: { + qs: { + access_token: auth.access_token, + }, + }, + errorMessages: { + 401: 'auth failed', + 404: 'room not found', + 429: 'rate limited by rooms host', + }, + }) + } + + async fetch({ roomAlias }) { + const splitAlias = roomAlias.split(':') + // A room alias can either be in the form #localpart:server or + // #localpart:server:port. In the latter case, it's wiser to skip the name + // resolution and use that value right away. + if (splitAlias.length < 2 || splitAlias.length > 3) { + throw new errors.InvalidParameter() + } + let host + if (splitAlias.length === 2) { + host = splitAlias[1] + try { + const addrs = await this.lookupMatrixHomeserver({ host }) + if (addrs.length) { + // The address we are given may be only to use for federation. + // Therefore we check if we can painlessly reach the client APIs at + // this address, and if not we don't do anything, and ignore the + // error, since host already holds the right value, and we expect this + // check to fail in some cases. + try { + await this.checkMatrixHomeserverClientAPI({ host: addrs[0].name }) + host = addrs[0].name + } catch (e) {} + } + } catch (e) { + // If the error is ENOTFOUND, it means that there is no SRV record for + // this server, and that we need to fall back on the value host already + // holds. + if (e.code !== 'ENOTFOUND') { + throw e + } } + } else { + host = splitAlias[2] + splitAlias[3] } let auth try { @@ -117,8 +152,9 @@ module.exports = class Matrix extends BaseJsonService { auth = await this.registerAccount({ host, guest: false }) } else throw e } + const lookup = await this.lookupRoomAlias({ host, roomAlias, auth }) const data = await this._requestJson({ - url: `https://${host}/_matrix/client/r0/rooms/${roomId}/state`, + url: `https://${host}/_matrix/client/r0/rooms/${lookup.room_id}/state`, schema: matrixStateSchema, options: { qs: { @@ -152,11 +188,8 @@ module.exports = class Matrix extends BaseJsonService { } } - async handle({ roomId, host, authServer }) { - const members = await this.fetch({ - host, - roomId: `${roomId}:${host}`, - }) + async handle({ roomAlias, authServer }) { + const members = await this.fetch({ roomAlias }) return this.constructor.render({ members }) } @@ -171,8 +204,8 @@ module.exports = class Matrix extends BaseJsonService { static get route() { return { base: 'matrix', - format: '([^/]+)/([^/]+)', - capture: ['roomId', 'host'], + format: '([^/]+)', + capture: ['roomAlias'], } } From 8453c2fbe4758f5f36dd7531c77ace58394cab29 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 Dec 2018 21:47:22 +0000 Subject: [PATCH 05/26] Bonus: Store access tokens in memory to avoid countless registrations --- services/matrix/matrix.service.js | 38 ++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 732c1a4ecee00..6ede266d3be31 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -55,6 +55,8 @@ const documentation = ` const srvPrefix = '_matrix._tcp.' const resolve = util.promisify(dns.resolveSrv) +const accessTokens = {} + module.exports = class Matrix extends BaseJsonService { async lookupMatrixHomeserver({ host }) { return resolve(srvPrefix + host) @@ -67,6 +69,24 @@ module.exports = class Matrix extends BaseJsonService { }) } + async retrieveAccessToken({ host }) { + if (accessTokens[host] === undefined) { + let auth + try { + auth = await this.registerAccount({ host, guest: true }) + } catch (e) { + if (e.prettyMessage === 'guests not allowed') { + // attempt fallback method + auth = await this.registerAccount({ host, guest: false }) + } else throw e + } + + accessTokens[host] = auth.access_token + } + + return accessTokens[host] + } + async registerAccount({ host, guest }) { return this._requestJson({ url: `https://${host}/_matrix/client/r0/register`, @@ -91,13 +111,13 @@ module.exports = class Matrix extends BaseJsonService { }) } - async lookupRoomAlias({ host, roomAlias, auth }) { + async lookupRoomAlias({ host, roomAlias, accessToken }) { return this._requestJson({ url: `https://${host}/_matrix/client/r0/directory/room/%23${roomAlias}`, schema: matrixAliasLookupSchema, options: { qs: { - access_token: auth.access_token, + access_token: accessToken, }, }, errorMessages: { @@ -143,22 +163,14 @@ module.exports = class Matrix extends BaseJsonService { } else { host = splitAlias[2] + splitAlias[3] } - let auth - try { - auth = await this.registerAccount({ host, guest: true }) - } catch (e) { - if (e.prettyMessage === 'guests not allowed') { - // attempt fallback method - auth = await this.registerAccount({ host, guest: false }) - } else throw e - } - const lookup = await this.lookupRoomAlias({ host, roomAlias, auth }) + const accessToken = await this.retrieveAccessToken({ host }) + const lookup = await this.lookupRoomAlias({ host, roomAlias, accessToken }) const data = await this._requestJson({ url: `https://${host}/_matrix/client/r0/rooms/${lookup.room_id}/state`, schema: matrixStateSchema, options: { qs: { - access_token: auth.access_token, + access_token: accessToken, }, }, errorMessages: { From a72bb2de1ddd201440667c0aef76948a4e6354c1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 Dec 2018 22:17:40 +0000 Subject: [PATCH 06/26] Update doc --- services/matrix/matrix.service.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 6ede266d3be31..47c542589c11b 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -44,10 +44,11 @@ const documentation = `
  • Select the desired room inside the Riot.im client
  • Click on the room settings button (gear icon) located near the top right of the client
    • -
  • Scroll to the very bottom of the settings page and look under the Advanced tab
    • -
  • You should see the Internal room ID with your rooms ID next to it (ex: !ltIjvaLydYAWZyihee:matrix.org)
    • -
  • Replace the IDs : with /
    • -
  • The final badge URL should look something like this /matrix/!ltIjvaLydYAWZyihee/matrix.org.svg
    • +
  • Scroll to the very bottom of the settings page and look under the Addresses section
    • +
  • You should see one or more room addresses (or aliases), which can be easily identified with their starting hash (#) character (ex: #twim:matrix.org)
    • +
  • If there is no address for this room, add one under Local addresses for this room
    • +
  • Remove the starting hash character (#)
    • +
  • The final badge URL should look something like this /matrix/twim:matrix.org.svg

` @@ -225,8 +226,8 @@ module.exports = class Matrix extends BaseJsonService { return [ { title: 'Matrix', - exampleUrl: '!ltIjvaLydYAWZyihee/matrix.org', - pattern: ':roomId/:host', + exampleUrl: 'twim:matrix.org', + pattern: ':roomAlias', staticExample: this.render({ members: 42 }), documentation, }, From 55b33b03b23edc6871eefcc23b5459ceb98727c8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 Dec 2018 23:24:42 +0000 Subject: [PATCH 07/26] Fix error message --- services/matrix/matrix.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 47c542589c11b..0d7262bbc1702 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -122,7 +122,7 @@ module.exports = class Matrix extends BaseJsonService { }, }, errorMessages: { - 401: 'auth failed', + 401: 'bad auth token', 404: 'room not found', 429: 'rate limited by rooms host', }, From a7468fdd067f459ec43a18e7cc2ecb28354e2bf2 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 13 Dec 2018 23:25:46 +0000 Subject: [PATCH 08/26] Add new test for alias + improve existing tests --- services/matrix/matrix.tester.js | 76 ++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 9 deletions(-) diff --git a/services/matrix/matrix.tester.js b/services/matrix/matrix.tester.js index 225b452577840..371276200bb9d 100644 --- a/services/matrix/matrix.tester.js +++ b/services/matrix/matrix.tester.js @@ -8,7 +8,7 @@ const t = new ServiceTester({ id: 'matrix', title: 'Matrix' }) module.exports = t t.create('get room state as guest') - .get('/ROOM/DUMMY.dumb.json?style=_shields_test') + .get('/ALIAS:DUMMY.dumb.json?style=_shields_test') .intercept(nock => nock('https://DUMMY.dumb/') .post('/_matrix/client/r0/register?kind=guest') @@ -18,6 +18,15 @@ t.create('get room state as guest') access_token: 'TOKEN', }) ) + .get( + '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + ) + .reply( + 200, + JSON.stringify({ + room_id: 'ROOM:DUMMY.dumb', + }) + ) .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') .reply( 200, @@ -68,14 +77,14 @@ t.create('get room state as guest') }) t.create('get room state as member (backup method)') - .get('/ROOM/DUMMY.dumb.json?style=_shields_test') + .get('/ALIAS:DUMMY.dumb.json?style=_shields_test') .intercept(nock => nock('https://DUMMY.dumb/') .post('/_matrix/client/r0/register?kind=guest') .reply( 403, JSON.stringify({ - errcode: 'M_GUEST_ACCESS_FORBIDDEN', // i think this is the right one + errcode: 'M_GUEST_ACCESS_FORBIDDEN', error: 'Guest access not allowed', }) ) @@ -86,6 +95,15 @@ t.create('get room state as member (backup method)') access_token: 'TOKEN', }) ) + .get( + '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + ) + .reply( + 200, + JSON.stringify({ + room_id: 'ROOM:DUMMY.dumb', + }) + ) .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') .reply( 200, @@ -136,7 +154,7 @@ t.create('get room state as member (backup method)') }) t.create('bad server or connection') - .get('/ROOM/DUMMY.dumb.json?style=_shields_test') + .get('/ALIAS:DUMMY.dumb.json?style=_shields_test') .networkOff() .expectJSON({ name: 'chat', @@ -145,7 +163,7 @@ t.create('bad server or connection') }) t.create('invalid room') - .get('/ROOM/DUMMY.dumb.json?style=_shields_test') + .get('/ALIAS:DUMMY.dumb.json?style=_shields_test') .intercept(nock => nock('https://DUMMY.dumb/') .post('/_matrix/client/r0/register?kind=guest') @@ -155,6 +173,15 @@ t.create('invalid room') access_token: 'TOKEN', }) ) + .get( + '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + ) + .reply( + 200, + JSON.stringify({ + room_id: 'ROOM:DUMMY.dumb', + }) + ) .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') .reply( 403, @@ -171,7 +198,7 @@ t.create('invalid room') }) t.create('invalid token') - .get('/ROOM/DUMMY.dumb.json?style=_shields_test') + .get('/ALIAS:DUMMY.dumb.json?style=_shields_test') .intercept(nock => nock('https://DUMMY.dumb/') .post('/_matrix/client/r0/register?kind=guest') @@ -181,7 +208,9 @@ t.create('invalid token') access_token: 'TOKEN', }) ) - .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') + .get( + '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + ) .reply( 401, JSON.stringify({ @@ -197,7 +226,7 @@ t.create('invalid token') }) t.create('unknown request') - .get('/ROOM/DUMMY.dumb.json?style=_shields_test') + .get('/ALIAS:DUMMY.dumb.json?style=_shields_test') .intercept(nock => nock('https://DUMMY.dumb/') .post('/_matrix/client/r0/register?kind=guest') @@ -222,8 +251,37 @@ t.create('unknown request') colorB: colorScheme.lightgray, }) +t.create('unknown alias') + .get('/ALIAS:DUMMY.dumb.json?style=_shields_test') + .intercept(nock => + nock('https://DUMMY.dumb/') + .post('/_matrix/client/r0/register?kind=guest') + .reply( + 200, + JSON.stringify({ + access_token: 'TOKEN', + }) + ) + .get( + '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + ) + .reply( + 404, + JSON.stringify({ + errcode: 'M_NOT_FOUND', + error: 'Room alias #ALIAS:DUMMY.dumb not found.', + }) + ) + ) + .expectJSON({ + name: 'chat', + value: 'room not found', + colorB: colorScheme.red, + }) + t.create('test on real matrix room for API compliance') - .get('/!ltIjvaLydYAWZyihee/matrix.org.json?style=_shields_test') + .get('/twim:matrix.org.json?style=_shields_test') + .timeout(10000) .expectJSONTypes( Joi.object().keys({ name: 'chat', From 48cff03719e9d1a88a7b7c05e8e4e15e08da5363 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Dec 2018 20:08:43 +0000 Subject: [PATCH 09/26] Use optional URL parameter to determine where to reach homeservers rather than resolving it --- services/matrix/matrix.service.js | 85 ++++++++++--------------------- 1 file changed, 27 insertions(+), 58 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 0d7262bbc1702..e849e4a574ef1 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -1,7 +1,5 @@ 'use strict' -const dns = require('dns') -const util = require('util') const Joi = require('joi') const BaseJsonService = require('../base-json') const errors = require('../errors') @@ -10,12 +8,6 @@ const matrixRegisterSchema = Joi.object({ access_token: Joi.string().required(), }).required() -const matrixClientVersionsSchema = Joi.object({ - versions: Joi.array() - .items(Joi.string().required()) - .required(), -}).required() - const matrixAliasLookupSchema = Joi.object({ room_id: Joi.string().required(), }) @@ -50,26 +42,18 @@ const documentation = `
  • Remove the starting hash character (#)
  • The final badge URL should look something like this /matrix/twim:matrix.org.svg
    • +
    + Some Matrix homeservers don't hold a server name matching where they live (e.g. if the homeserver example.com that created the room alias #mysuperroom:example.com lives at matrix.example.com). +
    + If that is the case of the homeserver that created the room alias used for generating the badge, you will need to add an extra parameter to the URL, by adding a forward slash followed by the server's FQDN (fully qualified domain name) between the room alias and the .svg extension. +
    + The final badge URL should then look something like this /matrix/mysuperroom:example.com/matrix.example.com.svg.

    ` -const srvPrefix = '_matrix._tcp.' -const resolve = util.promisify(dns.resolveSrv) - const accessTokens = {} module.exports = class Matrix extends BaseJsonService { - async lookupMatrixHomeserver({ host }) { - return resolve(srvPrefix + host) - } - - async checkMatrixHomeserverClientAPI({ host }) { - return this._requestJson({ - url: `https://${host}/_matrix/client/versions`, - schema: matrixClientVersionsSchema, - }) - } - async retrieveAccessToken({ host }) { if (accessTokens[host] === undefined) { let auth @@ -129,40 +113,25 @@ module.exports = class Matrix extends BaseJsonService { }) } - async fetch({ roomAlias }) { - const splitAlias = roomAlias.split(':') - // A room alias can either be in the form #localpart:server or - // #localpart:server:port. In the latter case, it's wiser to skip the name - // resolution and use that value right away. - if (splitAlias.length < 2 || splitAlias.length > 3) { - throw new errors.InvalidParameter() - } + async fetch({ roomAlias, serverFQDN }) { let host - if (splitAlias.length === 2) { - host = splitAlias[1] - try { - const addrs = await this.lookupMatrixHomeserver({ host }) - if (addrs.length) { - // The address we are given may be only to use for federation. - // Therefore we check if we can painlessly reach the client APIs at - // this address, and if not we don't do anything, and ignore the - // error, since host already holds the right value, and we expect this - // check to fail in some cases. - try { - await this.checkMatrixHomeserverClientAPI({ host: addrs[0].name }) - host = addrs[0].name - } catch (e) {} - } - } catch (e) { - // If the error is ENOTFOUND, it means that there is no SRV record for - // this server, and that we need to fall back on the value host already - // holds. - if (e.code !== 'ENOTFOUND') { - throw e - } + if (serverFQDN === undefined) { + const splitAlias = roomAlias.split(':') + // A room alias can either be in the form #localpart:server or + // #localpart:server:port. In the latter case, it's wiser to skip the name + // resolution and use that value right away. + switch (splitAlias.length) { + case 2: + host = splitAlias[1] + break + case 3: + host = splitAlias[2] + splitAlias[3] + break + default: + throw new errors.InvalidParameter() } } else { - host = splitAlias[2] + splitAlias[3] + host = serverFQDN } const accessToken = await this.retrieveAccessToken({ host }) const lookup = await this.lookupRoomAlias({ host, roomAlias, accessToken }) @@ -201,8 +170,8 @@ module.exports = class Matrix extends BaseJsonService { } } - async handle({ roomAlias, authServer }) { - const members = await this.fetch({ roomAlias }) + async handle({ roomAlias, serverFQDN }) { + const members = await this.fetch({ roomAlias, serverFQDN }) return this.constructor.render({ members }) } @@ -217,8 +186,8 @@ module.exports = class Matrix extends BaseJsonService { static get route() { return { base: 'matrix', - format: '([^/]+)', - capture: ['roomAlias'], + format: '([^/]+)(?:/([^/]+))?', + capture: ['roomAlias', 'serverFQDN'], } } @@ -227,7 +196,7 @@ module.exports = class Matrix extends BaseJsonService { { title: 'Matrix', exampleUrl: 'twim:matrix.org', - pattern: ':roomAlias', + pattern: ':roomAlias/:server_fqdn_optional', staticExample: this.render({ members: 42 }), documentation, }, From e17fd8e16f7761f0de56046915d194b93e9e92fb Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Dec 2018 22:02:11 +0000 Subject: [PATCH 10/26] Remove in-memory cache for access tokens --- services/matrix/matrix.service.js | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index e849e4a574ef1..abcf56f169dbf 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -51,25 +51,19 @@ const documentation = `

    ` -const accessTokens = {} - module.exports = class Matrix extends BaseJsonService { async retrieveAccessToken({ host }) { - if (accessTokens[host] === undefined) { - let auth - try { - auth = await this.registerAccount({ host, guest: true }) - } catch (e) { - if (e.prettyMessage === 'guests not allowed') { - // attempt fallback method - auth = await this.registerAccount({ host, guest: false }) - } else throw e - } - - accessTokens[host] = auth.access_token + let auth + try { + auth = await this.registerAccount({ host, guest: true }) + } catch (e) { + if (e.prettyMessage === 'guests not allowed') { + // attempt fallback method + auth = await this.registerAccount({ host, guest: false }) + } else throw e } - return accessTokens[host] + return auth.access_token } async registerAccount({ host, guest }) { From 3d9a5bb169691ae27892caea055024c59984b6c0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Dec 2018 22:07:32 +0000 Subject: [PATCH 11/26] Add test case for homeserver fqdn --- services/matrix/matrix.tester.js | 69 ++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/services/matrix/matrix.tester.js b/services/matrix/matrix.tester.js index 371276200bb9d..2a12d30aff5c2 100644 --- a/services/matrix/matrix.tester.js +++ b/services/matrix/matrix.tester.js @@ -279,6 +279,75 @@ t.create('unknown alias') colorB: colorScheme.red, }) +t.create('specify the homeserver fqdn') + .get('/ALIAS:DUMMY.dumb/matrix.DUMMY.dumb.json?style=_shields_test') + .intercept(nock => + nock('https://matrix.DUMMY.dumb/') + .post('/_matrix/client/r0/register?kind=guest') + .reply( + 200, + JSON.stringify({ + access_token: 'TOKEN', + }) + ) + .get( + '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + ) + .reply( + 200, + JSON.stringify({ + room_id: 'ROOM:DUMMY.dumb', + }) + ) + .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') + .reply( + 200, + JSON.stringify([ + { + // valid user 1 + type: 'm.room.member', + sender: '@user1:DUMMY.dumb', + state_key: '@user1:DUMMY.dumb', + content: { + membership: 'join', + }, + }, + { + // valid user 2 + type: 'm.room.member', + sender: '@user2:DUMMY.dumb', + state_key: '@user2:DUMMY.dumb', + content: { + membership: 'join', + }, + }, + { + // should exclude banned/invited/left members + type: 'm.room.member', + sender: '@user3:DUMMY.dumb', + state_key: '@user3:DUMMY.dumb', + content: { + membership: 'leave', + }, + }, + { + // exclude events like the room name + type: 'm.room.name', + sender: '@user4:DUMMY.dumb', + state_key: '@user4:DUMMY.dumb', + content: { + membership: 'fake room', + }, + }, + ]) + ) + ) + .expectJSON({ + name: 'chat', + value: '2 users', + colorB: colorScheme.brightgreen, + }) + t.create('test on real matrix room for API compliance') .get('/twim:matrix.org.json?style=_shields_test') .timeout(10000) From 3ae25b66d32587c60829848b4051a7171bbb8962 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Dec 2018 22:08:48 +0000 Subject: [PATCH 12/26] Rename room readibility test with a more explicit name --- services/matrix/matrix.tester.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/matrix/matrix.tester.js b/services/matrix/matrix.tester.js index 2a12d30aff5c2..2a06a4229183b 100644 --- a/services/matrix/matrix.tester.js +++ b/services/matrix/matrix.tester.js @@ -162,7 +162,7 @@ t.create('bad server or connection') colorB: colorScheme.lightgray, }) -t.create('invalid room') +t.create('non-world readable room') .get('/ALIAS:DUMMY.dumb.json?style=_shields_test') .intercept(nock => nock('https://DUMMY.dumb/') From 1c1fccd93d5f00d57bfc1649c6517e83d7e0f81e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Dec 2018 22:14:42 +0000 Subject: [PATCH 13/26] Fix unknown request test --- services/matrix/matrix.tester.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/services/matrix/matrix.tester.js b/services/matrix/matrix.tester.js index 2a06a4229183b..66e7c00239fef 100644 --- a/services/matrix/matrix.tester.js +++ b/services/matrix/matrix.tester.js @@ -236,6 +236,15 @@ t.create('unknown request') access_token: 'TOKEN', }) ) + .get( + '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + ) + .reply( + 200, + JSON.stringify({ + room_id: 'ROOM:DUMMY.dumb', + }) + ) .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') .reply( 400, From 3ac4a25c726451687a3bb197d021f156a3012339 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 16:58:02 +0000 Subject: [PATCH 14/26] Use namedParams instead of exampleUrl --- services/matrix/matrix.service.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index abcf56f169dbf..76f05329e6f3b 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -189,8 +189,10 @@ module.exports = class Matrix extends BaseJsonService { return [ { title: 'Matrix', - exampleUrl: 'twim:matrix.org', - pattern: ':roomAlias/:server_fqdn_optional', + namedParams: { + roomAlias: 'twim:matrix.org', + }, + pattern: ':roomAlias/:serverFqdnOptional', staticExample: this.render({ members: 42 }), documentation, }, From d655fa146346ce882ddb704b89b3dd719a074c85 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 17:05:20 +0000 Subject: [PATCH 15/26] Add serverFqdnOptional to example (as we run into errors otherwise) --- services/matrix/matrix.service.js | 1 + 1 file changed, 1 insertion(+) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 76f05329e6f3b..91fc896262609 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -191,6 +191,7 @@ module.exports = class Matrix extends BaseJsonService { title: 'Matrix', namedParams: { roomAlias: 'twim:matrix.org', + serverFqdnOptional: 'matrix.org', }, pattern: ':roomAlias/:serverFqdnOptional', staticExample: this.render({ members: 42 }), From 75b799d01f4d21914125d75db7a17d5887384161 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 17:09:13 +0000 Subject: [PATCH 16/26] Make error thrown when parsing the alias more explicit --- services/matrix/matrix.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 91fc896262609..9e3c7ed8c036a 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -122,7 +122,7 @@ module.exports = class Matrix extends BaseJsonService { host = splitAlias[2] + splitAlias[3] break default: - throw new errors.InvalidParameter() + throw new errors.InvalidParameter({ prettyMessage: 'invalid alias' }) } } else { host = serverFQDN From f9299103399c765619bd93081d68124c1e97f8f5 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 17:20:01 +0000 Subject: [PATCH 17/26] Remove mention of 'room host' --- services/matrix/matrix.service.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 9e3c7ed8c036a..6a2e905a5812f 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -85,7 +85,7 @@ module.exports = class Matrix extends BaseJsonService { errorMessages: { 401: 'auth failed', 403: 'guests not allowed', - 429: 'rate limited by rooms host', + 429: 'rate limited by remote server', }, }) } @@ -102,7 +102,7 @@ module.exports = class Matrix extends BaseJsonService { errorMessages: { 401: 'bad auth token', 404: 'room not found', - 429: 'rate limited by rooms host', + 429: 'rate limited by remote server', }, }) } From fad3adda24bb0a1397a3010e1b559971cf4db3e1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 17:28:03 +0000 Subject: [PATCH 18/26] Incorporate upstream's test suite improvements --- services/matrix/matrix.tester.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services/matrix/matrix.tester.js b/services/matrix/matrix.tester.js index 66e7c00239fef..5538572ada8ac 100644 --- a/services/matrix/matrix.tester.js +++ b/services/matrix/matrix.tester.js @@ -1,11 +1,9 @@ 'use strict' const Joi = require('joi') -const ServiceTester = require('../service-tester') const { colorScheme } = require('../test-helpers') -const t = new ServiceTester({ id: 'matrix', title: 'Matrix' }) -module.exports = t +const t = (module.exports = require('../create-service-tester')()) t.create('get room state as guest') .get('/ALIAS:DUMMY.dumb.json?style=_shields_test') From 2800387419a01f0fe7366a62125da685024cffac Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 17:36:05 +0000 Subject: [PATCH 19/26] Fix handling of port in alias --- services/matrix/matrix.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 6a2e905a5812f..e36304edb7f65 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -119,7 +119,7 @@ module.exports = class Matrix extends BaseJsonService { host = splitAlias[1] break case 3: - host = splitAlias[2] + splitAlias[3] + host = `${splitAlias[1]}:${splitAlias[2]}` break default: throw new errors.InvalidParameter({ prettyMessage: 'invalid alias' }) From 8bb5ebe30c4ac76668c36371112200f335612742 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 17:37:17 +0000 Subject: [PATCH 20/26] Add tests for invalid alias and port in alias --- services/matrix/matrix.tester.js | 79 ++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/services/matrix/matrix.tester.js b/services/matrix/matrix.tester.js index 5538572ada8ac..e7f9ae41034a7 100644 --- a/services/matrix/matrix.tester.js +++ b/services/matrix/matrix.tester.js @@ -286,6 +286,85 @@ t.create('unknown alias') colorB: colorScheme.red, }) +t.create('invalid alias') + .get('/ALIASDUMMY.dumb.json?style=_shields_test') + .expectJSON({ + name: 'chat', + value: 'invalid alias', + colorB: colorScheme.red, + }) + +t.create('server uses a custom port') + .get('/ALIAS:DUMMY.dumb:5555.json?style=_shields_test') + .intercept(nock => + nock('https://DUMMY.dumb:5555/') + .post('/_matrix/client/r0/register?kind=guest') + .reply( + 200, + JSON.stringify({ + access_token: 'TOKEN', + }) + ) + .get( + '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb:5555?access_token=TOKEN' + ) + .reply( + 200, + JSON.stringify({ + room_id: 'ROOM:DUMMY.dumb:5555', + }) + ) + .get( + '/_matrix/client/r0/rooms/ROOM:DUMMY.dumb:5555/state?access_token=TOKEN' + ) + .reply( + 200, + JSON.stringify([ + { + // valid user 1 + type: 'm.room.member', + sender: '@user1:DUMMY.dumb:5555', + state_key: '@user1:DUMMY.dumb:5555', + content: { + membership: 'join', + }, + }, + { + // valid user 2 + type: 'm.room.member', + sender: '@user2:DUMMY.dumb:5555', + state_key: '@user2:DUMMY.dumb:5555', + content: { + membership: 'join', + }, + }, + { + // should exclude banned/invited/left members + type: 'm.room.member', + sender: '@user3:DUMMY.dumb:5555', + state_key: '@user3:DUMMY.dumb:5555', + content: { + membership: 'leave', + }, + }, + { + // exclude events like the room name + type: 'm.room.name', + sender: '@user4:DUMMY.dumb:5555', + state_key: '@user4:DUMMY.dumb:5555', + content: { + membership: 'fake room', + }, + }, + ]) + ) + ) + .expectJSON({ + name: 'chat', + value: '2 users', + colorB: colorScheme.brightgreen, + }) + t.create('specify the homeserver fqdn') .get('/ALIAS:DUMMY.dumb/matrix.DUMMY.dumb.json?style=_shields_test') .intercept(nock => From 3ec0c77698fa65bea15619a4f31c7d464df96976 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 17:43:41 +0000 Subject: [PATCH 21/26] Update comment --- services/matrix/matrix.service.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index e36304edb7f65..6cd5e9151616b 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -112,8 +112,7 @@ module.exports = class Matrix extends BaseJsonService { if (serverFQDN === undefined) { const splitAlias = roomAlias.split(':') // A room alias can either be in the form #localpart:server or - // #localpart:server:port. In the latter case, it's wiser to skip the name - // resolution and use that value right away. + // #localpart:server:port. switch (splitAlias.length) { case 2: host = splitAlias[1] From 2a94b15e0c5f92bcb17333e187087d3accf9c6e1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 21:25:16 +0000 Subject: [PATCH 22/26] URL-encode room IDs and aliases --- services/matrix/matrix.service.js | 8 +++++-- services/matrix/matrix.tester.js | 40 +++++++++++++++++++------------ 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 6cd5e9151616b..b570e03be1a78 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -92,7 +92,9 @@ module.exports = class Matrix extends BaseJsonService { async lookupRoomAlias({ host, roomAlias, accessToken }) { return this._requestJson({ - url: `https://${host}/_matrix/client/r0/directory/room/%23${roomAlias}`, + url: `https://${host}/_matrix/client/r0/directory/room/${encodeURIComponent( + `#${roomAlias}` + )}`, schema: matrixAliasLookupSchema, options: { qs: { @@ -129,7 +131,9 @@ module.exports = class Matrix extends BaseJsonService { const accessToken = await this.retrieveAccessToken({ host }) const lookup = await this.lookupRoomAlias({ host, roomAlias, accessToken }) const data = await this._requestJson({ - url: `https://${host}/_matrix/client/r0/rooms/${lookup.room_id}/state`, + url: `https://${host}/_matrix/client/r0/rooms/${encodeURIComponent( + lookup.room_id + )}/state`, schema: matrixStateSchema, options: { qs: { diff --git a/services/matrix/matrix.tester.js b/services/matrix/matrix.tester.js index e7f9ae41034a7..e5f5556f98212 100644 --- a/services/matrix/matrix.tester.js +++ b/services/matrix/matrix.tester.js @@ -17,7 +17,7 @@ t.create('get room state as guest') }) ) .get( - '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + '/_matrix/client/r0/directory/room/%23ALIAS%3ADUMMY.dumb?access_token=TOKEN' ) .reply( 200, @@ -25,7 +25,9 @@ t.create('get room state as guest') room_id: 'ROOM:DUMMY.dumb', }) ) - .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') + .get( + '/_matrix/client/r0/rooms/ROOM%3ADUMMY.dumb/state?access_token=TOKEN' + ) .reply( 200, JSON.stringify([ @@ -94,7 +96,7 @@ t.create('get room state as member (backup method)') }) ) .get( - '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + '/_matrix/client/r0/directory/room/%23ALIAS%3ADUMMY.dumb?access_token=TOKEN' ) .reply( 200, @@ -102,7 +104,9 @@ t.create('get room state as member (backup method)') room_id: 'ROOM:DUMMY.dumb', }) ) - .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') + .get( + '/_matrix/client/r0/rooms/ROOM%3ADUMMY.dumb/state?access_token=TOKEN' + ) .reply( 200, JSON.stringify([ @@ -172,7 +176,7 @@ t.create('non-world readable room') }) ) .get( - '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + '/_matrix/client/r0/directory/room/%23ALIAS%3ADUMMY.dumb?access_token=TOKEN' ) .reply( 200, @@ -180,7 +184,9 @@ t.create('non-world readable room') room_id: 'ROOM:DUMMY.dumb', }) ) - .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') + .get( + '/_matrix/client/r0/rooms/ROOM%3ADUMMY.dumb/state?access_token=TOKEN' + ) .reply( 403, JSON.stringify({ @@ -207,7 +213,7 @@ t.create('invalid token') }) ) .get( - '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + '/_matrix/client/r0/directory/room/%23ALIAS%3ADUMMY.dumb?access_token=TOKEN' ) .reply( 401, @@ -235,7 +241,7 @@ t.create('unknown request') }) ) .get( - '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + '/_matrix/client/r0/directory/room/%23ALIAS%3ADUMMY.dumb?access_token=TOKEN' ) .reply( 200, @@ -243,7 +249,9 @@ t.create('unknown request') room_id: 'ROOM:DUMMY.dumb', }) ) - .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') + .get( + '/_matrix/client/r0/rooms/ROOM%3ADUMMY.dumb/state?access_token=TOKEN' + ) .reply( 400, JSON.stringify({ @@ -270,13 +278,13 @@ t.create('unknown alias') }) ) .get( - '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + '/_matrix/client/r0/directory/room/%23ALIAS%3ADUMMY.dumb?access_token=TOKEN' ) .reply( 404, JSON.stringify({ errcode: 'M_NOT_FOUND', - error: 'Room alias #ALIAS:DUMMY.dumb not found.', + error: 'Room alias #ALIAS%3ADUMMY.dumb not found.', }) ) ) @@ -306,7 +314,7 @@ t.create('server uses a custom port') }) ) .get( - '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb:5555?access_token=TOKEN' + '/_matrix/client/r0/directory/room/%23ALIAS%3ADUMMY.dumb%3A5555?access_token=TOKEN' ) .reply( 200, @@ -315,7 +323,7 @@ t.create('server uses a custom port') }) ) .get( - '/_matrix/client/r0/rooms/ROOM:DUMMY.dumb:5555/state?access_token=TOKEN' + '/_matrix/client/r0/rooms/ROOM%3ADUMMY.dumb%3A5555/state?access_token=TOKEN' ) .reply( 200, @@ -377,7 +385,7 @@ t.create('specify the homeserver fqdn') }) ) .get( - '/_matrix/client/r0/directory/room/%23ALIAS:DUMMY.dumb?access_token=TOKEN' + '/_matrix/client/r0/directory/room/%23ALIAS%3ADUMMY.dumb?access_token=TOKEN' ) .reply( 200, @@ -385,7 +393,9 @@ t.create('specify the homeserver fqdn') room_id: 'ROOM:DUMMY.dumb', }) ) - .get('/_matrix/client/r0/rooms/ROOM:DUMMY.dumb/state?access_token=TOKEN') + .get( + '/_matrix/client/r0/rooms/ROOM%3ADUMMY.dumb/state?access_token=TOKEN' + ) .reply( 200, JSON.stringify([ From 410015ea951608ec7e357ed6081d06f84cd816e7 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 21:37:49 +0000 Subject: [PATCH 23/26] Move server FQDN to query parameters --- services/matrix/matrix.service.js | 7 ++++--- services/matrix/matrix.tester.js | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index b570e03be1a78..2d49b197da2a1 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -167,7 +167,7 @@ module.exports = class Matrix extends BaseJsonService { } } - async handle({ roomAlias, serverFQDN }) { + async handle({ roomAlias }, { serverFQDN }) { const members = await this.fetch({ roomAlias, serverFQDN }) return this.constructor.render({ members }) } @@ -183,8 +183,9 @@ module.exports = class Matrix extends BaseJsonService { static get route() { return { base: 'matrix', - format: '([^/]+)(?:/([^/]+))?', - capture: ['roomAlias', 'serverFQDN'], + format: '([^/]+)', + capture: ['roomAlias'], + queryParams: ['serverFQDN'], } } diff --git a/services/matrix/matrix.tester.js b/services/matrix/matrix.tester.js index e5f5556f98212..c6bbded70e711 100644 --- a/services/matrix/matrix.tester.js +++ b/services/matrix/matrix.tester.js @@ -374,7 +374,9 @@ t.create('server uses a custom port') }) t.create('specify the homeserver fqdn') - .get('/ALIAS:DUMMY.dumb/matrix.DUMMY.dumb.json?style=_shields_test') + .get( + '/ALIAS:DUMMY.dumb.json?style=_shields_test&serverFQDN=matrix.DUMMY.dumb' + ) .intercept(nock => nock('https://matrix.DUMMY.dumb/') .post('/_matrix/client/r0/register?kind=guest') From 2d4ec570f5af4cf4f7ffb9e71f5c864093c0d1b8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 21:44:17 +0000 Subject: [PATCH 24/26] Update doc and examples with recent changes --- services/matrix/matrix.service.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 2d49b197da2a1..515ab6dbc7c05 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -45,9 +45,9 @@ const documentation = `
    Some Matrix homeservers don't hold a server name matching where they live (e.g. if the homeserver example.com that created the room alias #mysuperroom:example.com lives at matrix.example.com).
    - If that is the case of the homeserver that created the room alias used for generating the badge, you will need to add an extra parameter to the URL, by adding a forward slash followed by the server's FQDN (fully qualified domain name) between the room alias and the .svg extension. + If that is the case of the homeserver that created the room alias used for generating the badge, you will need to add the server's FQDN (fully qualified domain name) as a query parameter.
    - The final badge URL should then look something like this /matrix/mysuperroom:example.com/matrix.example.com.svg. + The final badge URL should then look something like this /matrix/mysuperroom:example.com.svg?serverFQDN=matrix.example.com.svg.

    ` @@ -193,11 +193,16 @@ module.exports = class Matrix extends BaseJsonService { return [ { title: 'Matrix', - namedParams: { - roomAlias: 'twim:matrix.org', - serverFqdnOptional: 'matrix.org', - }, - pattern: ':roomAlias/:serverFqdnOptional', + namedParams: { roomAlias: 'twim:matrix.org' }, + pattern: ':roomAlias', + staticExample: this.render({ members: 42 }), + documentation, + }, + { + title: 'Matrix', + namedParams: { roomAlias: 'twim:matrix.org' }, + queryParams: { serverFQDN: 'matrix.org' }, + pattern: ':roomAlias', staticExample: this.render({ members: 42 }), documentation, }, From 56db2a9d56cd00fd52157ab04491df65141ee3f8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 20 Dec 2018 21:48:00 +0000 Subject: [PATCH 25/26] Fix typo in doc --- services/matrix/matrix.service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index 515ab6dbc7c05..e846f766f893f 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -47,7 +47,7 @@ const documentation = `
    If that is the case of the homeserver that created the room alias used for generating the badge, you will need to add the server's FQDN (fully qualified domain name) as a query parameter.
    - The final badge URL should then look something like this /matrix/mysuperroom:example.com.svg?serverFQDN=matrix.example.com.svg. + The final badge URL should then look something like this /matrix/mysuperroom:example.com.svg?serverFQDN=matrix.example.com.

    ` From fcdbd8edc8a7fd9998958cc6ba3c68441cf3558e Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Thu, 20 Dec 2018 16:55:24 -0500 Subject: [PATCH 26/26] Validate query params + use snake case --- services/matrix/matrix.service.js | 16 ++++++++++++---- services/matrix/matrix.tester.js | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js index e846f766f893f..4c9b78114ede1 100644 --- a/services/matrix/matrix.service.js +++ b/services/matrix/matrix.service.js @@ -4,6 +4,10 @@ const Joi = require('joi') const BaseJsonService = require('../base-json') const errors = require('../errors') +const queryParamSchema = Joi.object({ + server_fqdn: Joi.string().hostname(), +}).required() + const matrixRegisterSchema = Joi.object({ access_token: Joi.string().required(), }).required() @@ -47,7 +51,7 @@ const documentation = `
    If that is the case of the homeserver that created the room alias used for generating the badge, you will need to add the server's FQDN (fully qualified domain name) as a query parameter.
    - The final badge URL should then look something like this /matrix/mysuperroom:example.com.svg?serverFQDN=matrix.example.com. + The final badge URL should then look something like this /matrix/mysuperroom:example.com.svg?server_fqdn=matrix.example.com.

    ` @@ -167,7 +171,11 @@ module.exports = class Matrix extends BaseJsonService { } } - async handle({ roomAlias }, { serverFQDN }) { + async handle({ roomAlias }, queryParams) { + const { server_fqdn: serverFQDN } = this.constructor._validateQueryParams( + queryParams, + queryParamSchema + ) const members = await this.fetch({ roomAlias, serverFQDN }) return this.constructor.render({ members }) } @@ -185,7 +193,7 @@ module.exports = class Matrix extends BaseJsonService { base: 'matrix', format: '([^/]+)', capture: ['roomAlias'], - queryParams: ['serverFQDN'], + queryParams: ['server_fqdn'], } } @@ -201,7 +209,7 @@ module.exports = class Matrix extends BaseJsonService { { title: 'Matrix', namedParams: { roomAlias: 'twim:matrix.org' }, - queryParams: { serverFQDN: 'matrix.org' }, + queryParams: { server_fqdn: 'matrix.org' }, pattern: ':roomAlias', staticExample: this.render({ members: 42 }), documentation, diff --git a/services/matrix/matrix.tester.js b/services/matrix/matrix.tester.js index c6bbded70e711..1bbf5915a597a 100644 --- a/services/matrix/matrix.tester.js +++ b/services/matrix/matrix.tester.js @@ -375,7 +375,7 @@ t.create('server uses a custom port') t.create('specify the homeserver fqdn') .get( - '/ALIAS:DUMMY.dumb.json?style=_shields_test&serverFQDN=matrix.DUMMY.dumb' + '/ALIAS:DUMMY.dumb.json?style=_shields_test&server_fqdn=matrix.DUMMY.dumb' ) .intercept(nock => nock('https://matrix.DUMMY.dumb/')