manymjolnir appservice #364

jesopo · 2022-08-30T13:15:44Z

No description provided.

src/appservice/Api.ts

src/appservice/AppService.ts

src/appservice/MjolnirManager.ts

mx-tester.yml

docs/appservice.md

Yoric · 2022-11-07T08:53:47Z

docs/appservice.md

+
+   `docker run -rm -v /mjolnir/data/path:/data matrixdotorg/mjolnir appservice -r -u "http://$HOST:$MATRIX_PORT" -f /data/config/mjolnir-registration.yaml`
+
+5. Invite the application service bot (specified in the registration) to the access control room you made earlier.


Nit: "made" => "created"

Also, I'd rephrase:

"Step 4 created an application service bot. You now need to invite it in the access control room that you have created in Step 1."

However, don't we need to start the application service (step 6) before inviting the bot?

We don't need to start the service before inviting the bot

The appservice doesn't need to be aware of the invite, only the homeserver

as the appservice will try joining anyways

docs/appservice.md

Yoric · 2022-11-07T11:29:22Z

src/appservice/datastore.ts

+export interface MjolnirRecord {
+    local_part: string,
+    owner: string,
+    management_room: string,


Could we clarify that it's a management_room_id?

Probably too late now, but we should follow up once schema management is sorted out, added to #415

Yoric · 2022-11-07T11:30:34Z

src/appservice/datastore.ts

+    }
+
+    public async store(mjolnirRecord: MjolnirRecord): Promise<void> {
+        await this.pgClient.query(


How does this behave if the connection to the client is lost at some point, i.e. if Postgres is upgraded while Mjölnir is on?

Probably explodes, how are you supposed to handle that? (genuine)

Generally, in my code, I use a connection pool. I drop the connection as soon as I stop using it and I request a connection from the pool when I need a new one. The connection pool is smart enough to reconnect if I have been disconnected. This limits the risks of exploding in flight.

Sounds like a followup issue?

test/appservice/integration/webAPITest.ts

test/appservice/utils/harness.ts

Yoric · 2022-11-07T11:41:19Z

test/appservice/utils/webAPIClient.ts

+    private constructor(
+        private readonly matrixClient: MatrixClient,
+        /**
+         * This will want refactoring later on since, well, for some reason our API is designed to accept the token in request body and not authorization header :( 


Certainly true, but that's not documentation :)

Is this something that can be done quickly?

Probably not #416

Yoric

Looks pretty good! I have a few questions but I feel that we're nearly there.

docs/appservice.md

src/appservice/AccessControl.ts

Yoric · 2022-11-14T16:15:28Z

src/appservice/Api.ts

+                try {
+                    response = JSON.parse(body);
+                } catch (e) {
+                    reject(null);


I don't see it documented?

src/appservice/Api.ts

Yoric · 2022-11-15T09:52:31Z

test/appservice/integration/provisionTest.ts

+        this.user = user;
+        const roomsInvitedTo: string[] = [];
+        await new Promise(async resolve => {
+            user.on('room.invite', (roomId: string) => {


I don't understand why we're using user to watch for a room.invite. Aren't we expecting the bot or a new virtual user to be invited?

The user gets invited to a management and policy room by the newly provisioned mjolnir

Yoric · 2022-11-15T09:53:05Z

test/appservice/integration/provisionTest.ts

+            user.on('room.invite', (roomId: string) => {
+                roomsInvitedTo.push(roomId)
+                // the appservice should invite it to a policy room and a management room.
+                if (roomsInvitedTo.length === 2) {


We're only checking the length of roomsInvitedTo? Not the actual rooms?

We check those later, it would be nice to describe the rooms we are looking for upfront, but I'm not sure matrix is introspective enough for that.

test/appservice/integration/provisionTest.ts

Yoric · 2022-11-15T09:53:45Z

test/appservice/integration/webAPITest.ts

+        const mjolnirDetails: CreateMjolnirResponse = await new Promise(async resolve => {
+            const mjolnirDetailsPromise = apiClient.createMjolnir(roomToProtectId);
+            user.on('room.invite', (roomId: string) => {
+                roomsInvitedTo.push(roomId)


Basically, same questions as in the previous test.

Yes, seems they are almost the same

test/appservice/utils/webAPIClient.ts

matrix-bot-sdk imporved the types for this

still needs hooking up when mjolnir restarts, and when the access control changes

It's much harder to forget to initialize the components now that you have to in order to construct them in the first place.

this means that the appsrvice bot is now called 'mjolnir-bot' by default which was easier than going through old code base and renaming

We could have used another Dockerfile for the appservice, extending the exising one but we decided not to because there would have been lots of fiddling around the entrypoint and logistics involved around adding a tag for it via github actions. Not to mention that this would be duplicating the image just to run it with a different binary. This solution is much simpler, backwards compatible, and conscious about the future.

#416.

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch from 4748926 to 99c9c37 Compare September 12, 2022 16:18

Yoric reviewed Sep 16, 2022

View reviewed changes

src/appservice/Api.ts Show resolved Hide resolved

src/appservice/AppService.ts Show resolved Hide resolved

src/appservice/MjolnirManager.ts Show resolved Hide resolved

src/appservice/MjolnirManager.ts Outdated Show resolved Hide resolved

Yoric reviewed Sep 19, 2022

View reviewed changes

mx-tester.yml Show resolved Hide resolved

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch from 4af1e5d to 839fe05 Compare September 30, 2022 15:56

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch 2 times, most recently from bd919e4 to c6def0b Compare October 19, 2022 14:31

jesopo force-pushed the gnuxie/appservice-mjolnir branch from c6def0b to bd919e4 Compare October 19, 2022 14:59

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch 5 times, most recently from 3057d05 to 2d40fdb Compare October 24, 2022 17:05

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch 3 times, most recently from 092044d to 818e4cf Compare November 4, 2022 13:02

Gnuxie marked this pull request as ready for review November 4, 2022 13:02

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch from b854954 to bd544ab Compare November 4, 2022 13:40

Gnuxie requested a review from Yoric November 4, 2022 13:43

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch from bd544ab to ec4cb2e Compare November 7, 2022 09:39

Yoric reviewed Nov 7, 2022

View reviewed changes

Gnuxie mentioned this pull request Nov 10, 2022

Postgres backend schema management in the appservice #415

Open

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch from 36671ea to b828501 Compare November 10, 2022 16:56

Gnuxie requested a review from Yoric November 10, 2022 16:58

Yoric approved these changes Nov 15, 2022

View reviewed changes

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch 2 times, most recently from 55a4b7f to f6b8f45 Compare November 15, 2022 17:00

Gnuxie and others added 4 commits November 15, 2022 17:05

AppService start

94040b0

move appservice bits

f68b511

update deps

b943a02

draft widget backend

c1d79a6

jesopo and others added 26 commits November 15, 2022 17:05

reconstructing mjolnirs after a reboot works!

8b1faf0

idk why we had nedb in there for matrix-appservice-bridge

9781b6e

tighter guard around room type in PolicyList

fb2868d

matrix-bot-sdk imporved the types for this

remove unnecessary import

383d3fe

enable esModuleInterop

a0da2e1

launch and use postgres in a container whilst using mx-tester

8771ca9

allow appservice to be configured

4c434c6

Add some much needed ergonomics to how the bridge is constructed.

c0ad6da

limited access control

074a16b

still needs hooking up when mjolnir restarts, and when the access control changes

Redesign initialization API of many mjolnir.

a03640c

It's much harder to forget to initialize the components now that you have to in order to construct them in the first place.

Shadowed name: 'request' in api.ts

94e1a89

regenerate mjolnir-registration in mx-tester

6d04807

Ammend config not to clash with existing CI

ab4655f

this means that the appsrvice bot is now called 'mjolnir-bot' by default which was easier than going through old code base and renaming

oopsie, this should also trigger when type isn't provided

7ad1720

Begin integration test suite for appservice.

689b5b9

Add some introductory docs for deploying the appservice.

4cd13f6

FIXUP doc bits idk

51e6016

Fix improper closing of resources in integration tests.

0bd666f

start testing the web api used by the widget

f9f7dcf

Tidy up the web API

e3270c9

Tidy up mjolnirManager

f27cb05

Document remaining issues.

73bf58e

Simplify API for getting a virtual user when provisioning mjolnir.

1d49414

appservice doc

d03441e

AccessControl doc

4d07b1b

Gnuxie force-pushed the gnuxie/appservice-mjolnir branch from f6b8f45 to 4d07b1b Compare November 15, 2022 17:06

Log errors when resolving openID tokens in web api.

3cbc495

#416.

Gnuxie merged commit 50f80f2 into main Nov 15, 2022

Gnuxie mentioned this pull request Jun 5, 2024

MSC4150: m.allow recommendation for moderation policy lists matrix-org/matrix-spec-proposals#4150

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

manymjolnir appservice #364

manymjolnir appservice #364

jesopo commented Aug 30, 2022

Yoric Nov 7, 2022

Gnuxie Nov 10, 2022

Gnuxie Nov 10, 2022

Gnuxie Nov 10, 2022

Yoric Nov 7, 2022

Gnuxie Nov 10, 2022

Yoric Nov 7, 2022

Gnuxie Nov 10, 2022

Yoric Nov 15, 2022

Gnuxie Nov 15, 2022

Yoric Nov 7, 2022

Gnuxie Nov 10, 2022

Yoric left a comment

Yoric Nov 14, 2022

Yoric Nov 15, 2022

Gnuxie Nov 15, 2022

Yoric Nov 15, 2022

Gnuxie Nov 15, 2022

Yoric Nov 15, 2022

Gnuxie Nov 15, 2022


		`docker run -rm -v /mjolnir/data/path:/data matrixdotorg/mjolnir appservice -r -u "http://$HOST:$MATRIX_PORT" -f /data/config/mjolnir-registration.yaml`

		5. Invite the application service bot (specified in the registration) to the access control room you made earlier.

manymjolnir appservice #364

manymjolnir appservice #364

Conversation

jesopo commented Aug 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yoric left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment