Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Feb 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/96.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Matrix profiles can now be viewed over the gateway
1 change: 1 addition & 0 deletions changelog.d/97.feature
Original file line number Diff line number Diff line change
@@ -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.
1 change: 0 additions & 1 deletion src/AutoRegistration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/GatewayHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
7 changes: 4 additions & 3 deletions src/xmppjs/PresenceCache.ts
Original file line number Diff line number Diff line change
@@ -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");

Expand Down Expand Up @@ -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,
Expand Down
12 changes: 8 additions & 4 deletions src/xmppjs/Stanzas.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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 ""; }
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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>`;
}
}

Expand Down
10 changes: 10 additions & 0 deletions src/xmppjs/StatusCodes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export enum XMPPStatusCode {
RoomNonAnonymous = "100",
SelfPresence = "110",
RoomLoggingEnabled = "170",
RoomLoggingDisabled = "171",
RoomNowNonAnonymous = "172",
SelfBanned = "301",
SelfKicked = "307",
SelfKickedTechnical = "333",
}
38 changes: 26 additions & 12 deletions src/xmppjs/XJSGateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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");

Expand All @@ -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();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
41 changes: 21 additions & 20 deletions src/xmppjs/XJSInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down Expand Up @@ -253,13 +256,15 @@ export class XmppJsInstance extends EventEmitter implements IBifrostInstance {
});
}

public getAccountForJid(aJid: JID): XmppJsAccount|undefined {
log.debug(aJid);
public getAccountForJid(aJid: JID): {mxId: string}|undefined {
const gatewayMxid = this.gateway?.getMatrixIDForJID(`${aJid.local}@${aJid.domain}`, aJid);
if (gatewayMxid) {
return {mxId: gatewayMxid};
}
if (aJid.domain === this.myAddress.domain) {
log.debug(aJid.local, [...this.accounts.keys()]);
return this.accounts.get(aJid.toString());
}
// TODO: Handle MUC based JIDs?
return;
}

Expand Down Expand Up @@ -453,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}`;
Expand Down Expand Up @@ -548,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
Expand All @@ -576,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());
}
}

Expand All @@ -597,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);
Expand All @@ -608,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,
Expand Down Expand Up @@ -651,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
Expand Down
22 changes: 7 additions & 15 deletions test/xmppjs/test_Stanzas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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);
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions test/xmppjs/test_XJSAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe("XJSAccount", () => {
hId: "12345",
html: "",
body: "Hello!",
markable: true,
attachments: [],
});
});
Expand Down
Loading