Migrate the ban-sync component from matrix-appservice-irc #459
Conversation
src/components/ban-sync.ts
Outdated
|
|
||
| } | ||
|
|
||
| public async getHomeserverProperties(serverName: string) { |
There was a problem hiding this comment.
Could use jsdoc and return type.
src/components/ban-sync.ts
Outdated
| public async getHomeserverProperties(serverName: string) { | ||
| const hsData = this.homeserverPropertiesCache.get(serverName); | ||
| // Slightly fuzz the ttl. | ||
| const ttl = CACHE_HOMESERVER_PROPERTIES_FOR_MS + (Math.random()*60000); |
There was a problem hiding this comment.
I think this is missing a Date.now()?
src/components/ban-sync.ts
Outdated
| const { url } = await this.hostResolver.resolveMatrixServer(serverName); | ||
| const registrationResponse = await axios.post(new URL('/_matrix/client/v3/register', url).toString(), { }, { }); | ||
|
|
||
| let openReg = RegistrationStatus.Unknown; |
There was a problem hiding this comment.
I think it would help readability to put this all this logic in a separate method which only handles returning a RegistrationStatus. Also that way it could return instead of mutating openReg.
| return hsProps; | ||
| } | ||
|
|
||
| public async syncRules(intent: Intent) { |
There was a problem hiding this comment.
Could use a jsdoc and return type.
src/components/ban-sync.ts
Outdated
| } | ||
| else { | ||
| // Check the flows | ||
| for (const flow of flows) { |
There was a problem hiding this comment.
Finding this loop a bit hard to follow, I think it could be more straightforward with a .map of flow stages to RegistrationStatus and then taking the minimum?
Just a suggestion feel free to ignore :)
src/components/ban-sync.ts
Outdated
| } | ||
|
|
||
| const { url } = await this.hostResolver.resolveMatrixServer(serverName); | ||
| const registrationResponse = await axios.post(new URL('/_matrix/client/v3/register', url).toString(), { }, { }); |
There was a problem hiding this comment.
What about older homeservers? If old homeservers only expose /_matrix/client/r0/register, will the bridge allow them to stay despite having open reg?
There was a problem hiding this comment.
Fair point, we'll include those.
src/components/ban-sync.ts
Outdated
| else if (flows.length === 0) { | ||
| // No available flows, so closed. | ||
| openReg = RegistrationStatus.Closed; | ||
| } |
There was a problem hiding this comment.
In the case of GNOME, I tried
>>> r = requests.post("https://gnome.modular.im/_matrix/client/v3/register", json={})
>>> r.json()
{'session': 'lpztxwMUAyTzgzdLHWhvwGtm', 'flows': [{'stages': ['m.login.recaptcha', 'm.login.terms', 'm.login.email.identity']}], 'params': {'m.login.recaptcha': {'public_key': '6LcgI54UAAAAABGdGmruw6DdOocFpYVdjYBRe4zb'}, 'm.login.terms': {'policies': {'privacy_policy': {'version': '1.0', 'en': {'name': 'Privacy Policy', 'url': 'https://gnome.modular.im/_matrix/consent?v=1.0'}}}}}}
>>> r.json()["flows"]
[{'stages': ['m.login.recaptcha', 'm.login.terms', 'm.login.email.identity']}]
It looks like the HS has open registrations, but in practice it's restricted to xxx@gnome.org email addresses. Is there a way to detect that remotely?
https://github.com/matrix-org/matrix-appservice-irc/blob/develop/src/bridge/MatrixBanSync.ts never really needed to live in the IRC bridge and in practice might be useful for other bridges.
I've been cheeky here and added an additional feature to check for open registration in the code.
The tests still need porting.