Merge pull request #97 from matrix-org/hs/no-more-anonymous-users-via…

…-gateway

Stop XMPP users (and matrix users) from appearing anonymous over the gateway

changelog.d/97.feature

@@ -0,0 +1 @@
+XMPP and Matrix users are no longer anonymous over the gateway. This is to keep in line with Matrix's own identity visibility.

src/AutoRegistration.ts

@@ -127,7 +127,6 @@ export class AutoRegistration {
                 throw new Error("Protocol unsupported");
             }
             // We assume the caller has already validated this.
-            const proto = this.protoInstance.getProtocol(protocol)!;
             const step = this.autoRegConfig.protocolSteps![protocol];
             return this.generateParameters(step.parameters, mxId);
     }

src/GatewayHandler.ts

@@ -141,7 +141,7 @@ export class GatewayHandler {
         const room = await this.getVirtualRoom(roomid, this.bridge.getIntent());
         log.info(`Reconnecting ${mxid} to ${roomid}`);
         const user = (await this.store.getRemoteUsersFromMxId(mxid))[0];
-        if (!user) {
+        if (!user || !user.extraData) {
             log.warn("Cannot reconnect a user without a remote user stored");
             return;
         }

src/xmppjs/PresenceCache.ts

@@ -1,6 +1,7 @@
 import * as xml from "@xmpp/xml";
 import * as jid from "@xmpp/jid";
 import { Logging } from "matrix-appservice-bridge";
+import { XMPPStatusCode } from "./StatusCodes";
 
 const log = Logging.get("PresenceCache");
 
@@ -91,9 +92,9 @@ export class PresenceCache {
             return;
         }
         const mucUser = stanza.getChild("x", "http://jabber.org/protocol/muc#user");
-        const isSelf = !!(mucUser && mucUser.getChildByAttr("code", "110"));
-        const isKick = !!(mucUser && mucUser.getChildByAttr("code", "307"));
-        const isTechnicalRemoval = !!(mucUser && mucUser.getChildByAttr("code", "333"));
+        const isSelf = !!(mucUser && mucUser.getChildByAttr("code", XMPPStatusCode.SelfPresence));
+        const isKick = !!(mucUser && mucUser.getChildByAttr("code", XMPPStatusCode.SelfKicked));
+        const isTechnicalRemoval = !!(mucUser && mucUser.getChildByAttr("code", XMPPStatusCode.SelfKickedTechnical));
         const newUser = !this.presence.get(from.toString());
         const currentPresence = this.presence.get(from.toString()) || {
             resource: from.resource,

src/xmppjs/Stanzas.ts

@@ -1,6 +1,7 @@
 import * as uuid from "uuid/v4";
 import * as he from "he";
 import { IBasicProtocolMessage } from "../MessageFormatter";
+import { XMPPStatusCode } from "./StatusCodes";
 
 export interface IStza {
     type: string;
@@ -74,7 +75,7 @@ export class StzaPresence extends StzaBase {
 }
 
 export class StzaPresenceItem extends StzaPresence {
-    protected statusCodes: Set<string>;
+    public statusCodes: Set<XMPPStatusCode>;
     constructor(
         from: string,
         to: string,
@@ -91,7 +92,7 @@ export class StzaPresenceItem extends StzaPresence {
     }
 
     set self(isSelf: boolean) {
-        this.statusCodes[isSelf ? "add" : "delete"]("110");
+        this.statusCodes[isSelf ? "add" : "delete"](XMPPStatusCode.SelfPresence);
     }
 
     get itemContents(): string { return ""; }
@@ -165,7 +166,7 @@ export class StzaPresenceKick extends StzaPresenceItem {
         self: boolean = false,
     ) {
         super(from, to, undefined, "none", "none", self, undefined, "unavailable");
-        this.statusCodes.add("307");
+        this.statusCodes.add(XMPPStatusCode.SelfKicked);
     }
 
     get itemContents(): string {
@@ -177,6 +178,7 @@ export class StzaPresenceKick extends StzaPresenceItem {
 export class StzaMessage extends StzaBase {
     public html: string = "";
     public body: string = "";
+    public markable: boolean = true;
     public attachments: string[] = [];
     constructor(
         from: string,
@@ -215,8 +217,10 @@ export class StzaMessage extends StzaBase {
             // For reasons unclear to me, XMPP reccomend we make the body == attachment url to make them show up inline.
             this.body = this.attachments[0];
         }
+        // XEP-0333
+        const markable = this.markable ? "<markable xmlns='urn:xmpp:chat-markers:0'/>" : "";
         return `<message from="${this.from}" to="${this.to}" id="${this.id}" ${type}>`
-             + `${this.html}<body>${he.encode(this.body)}</body>${attachments}</message>`;
+             + `${this.html}<body>${he.encode(this.body)}</body>${attachments}${markable}</message>`;
     }
 }
 

src/xmppjs/StatusCodes.ts

@@ -0,0 +1,10 @@
+export enum XMPPStatusCode {
+    RoomNonAnonymous = "100",
+    SelfPresence = "110",
+    RoomLoggingEnabled = "170",
+    RoomLoggingDisabled = "171",
+    RoomNowNonAnonymous = "172",
+    SelfBanned = "301",
+    SelfKicked = "307",
+    SelfKickedTechnical = "333",
+}

src/xmppjs/XJSGateway.ts

@@ -4,7 +4,6 @@ import { jid, JID } from "@xmpp/jid";
 import { Logging } from "matrix-appservice-bridge";
 import { IConfigBridge } from "../Config";
 import { IBasicProtocolMessage } from "..//MessageFormatter";
-import { Metrics } from "../Metrics";
 import { IGatewayJoin, IUserStateChanged, IStoreRemoteUser, IUserInfo } from "../bifrost/Events";
 import { IGatewayRoom } from "../bifrost/Gateway";
 import { PresenceCache } from "./PresenceCache";
@@ -14,6 +13,8 @@ import { StzaPresenceItem, StzaMessage, StzaMessageSubject,
     StzaPresenceError, StzaBase, StzaPresenceKick } from "./Stanzas";
 import { IGateway } from "../bifrost/Gateway";
 import { GatewayMUCMembership, IGatewayMemberXmpp, IGatewayMemberMatrix } from "./GatewayMUCMembership";
+import { XMPPStatusCode } from "./StatusCodes";
+import { AutoRegistration } from "../AutoRegistration";
 
 const log = Logging.get("XmppJsGateway");
 
@@ -30,7 +31,7 @@ export class XmppJsGateway implements IGateway {
     private presenceCache: PresenceCache;
     // Storing every XMPP user and their anonymous.
     private members: GatewayMUCMembership;
-    constructor(private xmpp: XmppJsInstance, private config: IConfigBridge) {
+    constructor(private xmpp: XmppJsInstance, private registration: AutoRegistration, private config: IConfigBridge) {
         this.roomHistory = new Map();
         this.stanzaCache = new Map();
         this.members = new GatewayMUCMembership();
@@ -78,7 +79,7 @@ export class XmppJsGateway implements IGateway {
                     protocol_id: XMPP_PROTOCOL.id,
                     username: convName,
                 },
-                sender: member.anonymousJid.toString(),
+                sender: member.realJid.toString(),
                 state: "left",
                 kicker,
                 reason: wasKicked ? wasKicked.reason : delta.status!.status,
@@ -350,29 +351,42 @@ export class XmppJsGateway implements IGateway {
                     log.warn("Drain didn't arrive, oh well");
                 }
             }
+            let realJid;
+            if ((member as IGatewayMemberXmpp).realJid) {
+                realJid = (member as IGatewayMemberXmpp).realJid.toString();
+            } else {
+                realJid = this.registration.generateParametersFor(
+                    XMPP_PROTOCOL.id, (member as IGatewayMemberMatrix).matrixId,
+                ).username;
+            }
             this.xmpp.xmppSend(
                 new StzaPresenceItem(
                     member.anonymousJid.toString(),
                     stanza.attrs.from,
                     undefined,
                     "member",
                     "participant",
+                    false,
+                    realJid,
                 ),
             );
         }
 
         log.debug("Emitting membership of self");
         // 2. self presence
-        this.xmpp.xmppSend(
-            new StzaPresenceItem(
-                stanza.attrs.to,
-                stanza.attrs.from,
-                undefined,
-                "member",
-                "participant",
-                true,
-            ),
+        const selfPresence = new StzaPresenceItem(
+            stanza.attrs.to,
+            stanza.attrs.from,
+            undefined,
+            "member",
+            "participant",
+            true,
         );
+
+        // Matrix is non-anon, and matrix logs.
+        selfPresence.statusCodes.add(XMPPStatusCode.RoomNonAnonymous);
+        selfPresence.statusCodes.add(XMPPStatusCode.RoomLoggingEnabled);
+        this.xmpp.xmppSend(selfPresence);
         this.reflectXMPPMessage(chatName, x("presence", {
                 from: stanza.attrs.from,
                 to: null,

src/xmppjs/XJSInstance.ts

@@ -173,7 +173,10 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
             throw Error("Missing opts for xmpp: service, domain, password");
         }
         if (this.config.portals.enableGateway === true) {
-            this.xmppGateway = new XmppJsGateway(this, this.config.bridge);
+            if (!this.autoRegister) {
+                throw Error("Autoregistration must be enabled for gateways to work!");
+            }
+            this.xmppGateway = new XmppJsGateway(this, this.autoRegister, this.config.bridge);
         }
         this.defaultRes = opts.defaultResource ? opts.defaultResource : "matrix-bridge";
         log.info(`Starting new XMPP component instance to ${opts.service} using domain ${opts.domain}`);
@@ -455,13 +458,9 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
                     log.warn(`Message could not be sent, not forwarding to Matrix`);
                     return;
                 }
-                stanza.attrs.from = this.gateway!.getAnonIDForJID(
-                    convName, stanza.attrs.from) || false;
-                if (stanza.attrs.from === false) {
-                    log.warn(`Couldn't find a user for ${from.toString()} reflect message with, dropping`);
-                    return;
-                }
-                from = jid(stanza.attrs.from);
+                // We deliberately do not anonymize the JID here.
+                // We do however strip the resource
+                from = jid(`${from.local}@${from.domain}`);
             } else {
                 // This is a PM, then.
                 convName = `${to.local}@${to.domain}`;
@@ -550,20 +549,20 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
                         protocol_id: XMPP_PROTOCOL.id,
                         username: localAcct.remoteId,
                     },
-                    sender: stanza.attrs.from,
+                    sender: from.toString(),
                     typing: chatState.is("composing"),
                 } as IChatTyping);
             }
 
             if (chatState.is("active")) {
                 // TODO: Should this expire.
-                this.activeMUCUsers.add(stanza.attrs.from);
+                this.activeMUCUsers.add(from.toString());
                 const readMsg = this.lastMessageInMUC.get(convName);
                 if (readMsg) {
-                    log.info(`${stanza.attrs.from} became active, updating RR with ${readMsg.id}`);
+                    log.info(`${from.toString()} became active, updating RR with ${readMsg.id}`);
                     this.emit("read-receipt", {
                         eventName: "read-receipt",
-                        sender: stanza.attrs.from,
+                        sender: from.toString(),
                         messageId: readMsg.id,
                         conv: {
                             // Don't include the handle
@@ -578,8 +577,8 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
                     } as IChatReadReceipt);
                 }
             } else if (chatState.is("inactive")) {
-                log.info(`${stanza.attrs.from} became inactive`);
-                this.activeMUCUsers.delete(stanza.attrs.from);
+                log.info(`${from.toString()} became inactive`);
+                this.activeMUCUsers.delete(from.toString());
             }
         }
 
@@ -599,7 +598,7 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
                     protocol_id: XMPP_PROTOCOL.id,
                     username: localAcct.remoteId,
                 },
-                sender: stanza.attrs.from,
+                sender: from.toString(),
                 string: subject,
                 isGateway: false,
             } as IChatStringState);
@@ -610,7 +609,7 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
             log.debug("Don't know how to handle a message without children");
             return;
         }
-        this.handleTextMessage(stanza, localAcct, from, convName, alias != null);
+        return this.handleTextMessage(stanza, localAcct, from, convName, alias != null);
     }
 
     private handleTextMessage(stanza: Element, localAcct: XmppJsAccount, from: JID,
@@ -653,7 +652,7 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
             log.debug("Emitting group message", message);
             this.emit("received-chat-msg", {
                 eventName: "received-chat-msg",
-                sender: stanza.attrs.from,
+                sender: from.toString(),
                 message,
                 conv: {
                     // Don't include the handle

test/xmppjs/test_Stanzas.ts

@@ -57,23 +57,13 @@ describe("Stanzas", () => {
         });
     });
     describe("StzaMessage", () => {
-        it("should create a valid stanza", () => {
-            const xml = new StzaMessageSubject("foo@bar", "baz@bar", "someid", "This is a subject").xml;
-            assertXML(xml);
-            expect(xml).to.equal(
-                "<message from=\"foo@bar\" to=\"baz@bar\" id=\"someid\" type='groupchat'>"
-                + "<subject>This is a subject</subject></message>",
-            );
-        });
-    });
-    describe("StzaMessageSubject", () => {
         it("should create a valid stanza for a simple plain message", () => {
             const stanza = new StzaMessage("foo@bar", "baz@bar", "someid", "groupchat");
             stanza.body = "Viva la matrix̭";
             assertXML(stanza.xml);
             expect(stanza.xml).to.equal(
                 "<message from=\"foo@bar\" to=\"baz@bar\" id=\"someid\" type='groupchat'>"
-                + "<body>Viva la matrix&#x32D;</body></message>",
+                + "<body>Viva la matrix&#x32D;</body><markable xmlns='urn:xmpp:chat-markers:0'/></message>",
             );
         });
         it("should create a valid stanza for a html message", () => {
@@ -83,7 +73,8 @@ describe("Stanzas", () => {
             assertXML(stanza.xml);
             expect(stanza.xml).to.equal(
                 "<message from=\"foo@bar\" to=\"baz@bar\" id=\"someid\" type='groupchat'><html><p>"
-                + "<strong>Viva la</strong> matrix&#x32D;</p></html><body>Viva la matrix&#x32D;</body></message>",
+                + "<strong>Viva la</strong> matrix&#x32D;</p></html><body>Viva la matrix&#x32D;</body>"
+                + "<markable xmlns='urn:xmpp:chat-markers:0'/></message>",
             );
         });
         it("should create a valid stanza for a message with attachments", () => {
@@ -95,11 +86,12 @@ describe("Stanzas", () => {
             expect(stanza.xml).to.equal(
                 "<message from=\"foo@bar\" to=\"baz@bar\" id=\"someid\" type='groupchat'><html><p>"
                 + "<strong>Viva la</strong> matrix&#x32D;</p></html><body>http://matrix.org</body>"
-                + "<x xmlns='jabber:x:oob'><url>http://matrix.org</url></x></message>",
+                + "<x xmlns='jabber:x:oob'><url>http://matrix.org</url></x>"
+                + "<markable xmlns='urn:xmpp:chat-markers:0'/></message>",
             );
         });
     });
-    describe("StzaMessage", () => {
+    describe("StzaMessageSubject", () => {
         it("should create a valid stanza", () => {
             const xml = new StzaMessageSubject("foo@bar", "baz@bar", "someid", "This is a subject").xml;
             assertXML(xml);
@@ -109,7 +101,7 @@ describe("Stanzas", () => {
             );
         });
     });
-    describe("StzaMessage", () => {
+    describe("SztaIqError", () => {
         it("should create a valid stanza", () => {
             const xml = new SztaIqError("foo@bar", "baz@bar", "someid", "cancel", null, "not-acceptable", "foo").xml;
             assertXML(xml);

test/xmppjs/test_XJSAccount.ts

@@ -45,6 +45,7 @@ describe("XJSAccount", () => {
                 hId: "12345",
                 html: "",
                 body: "Hello!",
+                markable: true,
                 attachments: [],
             });
         });