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

client: Include from in the stream opening and stop resetting jid on disconnect #1039

Merged
merged 5 commits into from
Dec 30, 2024
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
10 changes: 8 additions & 2 deletions packages/client-core/lib/Client.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,14 @@ class Client extends Connection {
return this.Transport.prototype.socketParameters(...args);
}

header(...args) {
return this.Transport.prototype.header(...args);
header(headerElement, ...args) {
// if the client knows the XMPP identity then it SHOULD include the 'from' attribute
// after the confidentiality and integrity of the stream are protected via TLS
// or an equivalent security layer.
// https://xmpp.org/rfcs/rfc6120.html#rfc.section.4.7.1
const from = this.socket?.isSecure() && this.jid?.bare().toString();
if (from) headerElement.attrs.from = from;
return this.Transport.prototype.header(headerElement, ...args);
}

headerElement(...args) {
Expand Down
4 changes: 3 additions & 1 deletion packages/client-core/src/bind2/bind2.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import xml from "@xmpp/xml";

const NS_BIND = "urn:xmpp:bind:0";

export default function bind2({ sasl2 }, tag) {
export default function bind2({ sasl2, entity }, tag) {
const features = new Map();

sasl2.use(
Expand All @@ -22,6 +22,8 @@ export default function bind2({ sasl2 }, tag) {
);
},
(element) => {
if (!element.is("bound")) return;
entity._ready(false);
for (const child of element.getChildElements()) {
const feature = features.get(child.getNS());
feature?.[1]?.(child);
Expand Down
29 changes: 29 additions & 0 deletions packages/client-core/test/Client.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Client from "../lib/Client.js";
import { JID } from "@xmpp/test";

test("_findTransport", () => {
class Transport {
Expand All @@ -21,3 +22,31 @@ test("_findTransport", () => {
expect(entity._findTransport("b")).toBe(undefined);
expect(entity._findTransport("c")).toBe(undefined);
});

test("header", () => {
class Transport {
header(el) {
return el;
}
}

const entity = new Client();
entity.Transport = Transport;
entity.socket = {};

entity.jid = null;
entity.socket.isSecure = () => false;
expect(entity.header(<foo />)).toEqual(<foo />);

entity.jid = null;
entity.socket.isSecure = () => true;
expect(entity.header(<foo />)).toEqual(<foo />);

entity.jid = new JID("foo@bar/example");
entity.socket.isSecure = () => false;
expect(entity.header(<foo />)).toEqual(<foo />);

entity.jid = new JID("foo@bar/example");
entity.socket.isSecure = () => true;
expect(entity.header(<foo />)).toEqual(<foo from="foo@bar" />);
});
1 change: 1 addition & 0 deletions packages/client/example.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import debug from "@xmpp/debug";
const xmpp = client({
service: "ws://localhost:5280/xmpp-websocket",
// service: "xmpps://localhost:5223",
// service: "xmpp://localhost:5222",
domain: "localhost",
resource: "example",
username: "username",
Expand Down
11 changes: 4 additions & 7 deletions packages/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import _starttls from "@xmpp/starttls";
import _sasl2 from "@xmpp/sasl2";
import _sasl from "@xmpp/sasl";
import _resourceBinding from "@xmpp/resource-binding";
import _sessionEstablishment from "@xmpp/session-establishment";
import _streamManagement from "@xmpp/stream-management";
import _bind2 from "@xmpp/client-core/src/bind2/bind2.js";

Expand All @@ -34,6 +33,9 @@ function client(options = {}) {
}

const entity = new Client(params);
if (username && params.domain) {
entity.jid = jid(username, params.domain);
}

const reconnect = _reconnect({ entity });
const websocket = _websocket({ entity });
Expand Down Expand Up @@ -62,7 +64,7 @@ function client(options = {}) {
);

// SASL2 inline features
const bind2 = _bind2({ sasl2 }, resource);
const bind2 = _bind2({ sasl2, entity }, resource);

// Stream features - order matters and define priority
const sasl = _sasl(
Expand All @@ -80,10 +82,6 @@ function client(options = {}) {
{ iqCaller, streamFeatures },
resource,
);
const sessionEstablishment = _sessionEstablishment({
iqCaller,
streamFeatures,
});

return Object.assign(entity, {
entity,
Expand All @@ -101,7 +99,6 @@ function client(options = {}) {
sasl2,
sasl,
resourceBinding,
sessionEstablishment,
streamManagement,
mechanisms,
bind2,
Expand Down
2 changes: 1 addition & 1 deletion packages/component-core/lib/Component.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class Component extends Connection {
}

this._jid(this.options.domain);
this._status("online", this.jid);
this._ready(false);
}
}

Expand Down
7 changes: 7 additions & 0 deletions packages/connection-tcp/Socket.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { Socket as TCPSocket } from "net";

export default class Socket extends TCPSocket {
isSecure() {
return false;
}
}
2 changes: 1 addition & 1 deletion packages/connection-tcp/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Socket } from "net";
import Socket from "./Socket.js";
import Connection from "@xmpp/connection";
import { Parser } from "@xmpp/xml";
import { parseURI } from "@xmpp/connection/lib/util.js";
Expand Down
4 changes: 2 additions & 2 deletions packages/connection-tcp/test/Connection.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _Connection from "@xmpp/connection";
import Connection from "../index.js";
import Socket from "../Socket.js";

import net from "net";
import xml from "@xmpp/xml";

const NS_STREAM = "http://etherx.jabber.org/streams";
Expand All @@ -14,7 +14,7 @@ test("new Connection()", () => {

test("Socket", () => {
const conn = new Connection();
expect(conn.Socket).toBe(net.Socket);
expect(conn.Socket).toBe(Socket);
});

test("NS", () => {
Expand Down
12 changes: 12 additions & 0 deletions packages/connection-tcp/test/Socket.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import net from "node:net";
import Socket from "../Socket.js";

test("isSecure()", () => {
const socket = new Socket();
expect(socket.isSecure()).toBe(false);
});

test("instance of net.Socket", () => {
const socket = new Socket();
expect(socket).toBeInstanceOf(net.Socket);
});
11 changes: 10 additions & 1 deletion packages/connection/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ class Connection extends EventEmitter {
}

_reset() {
this.jid = null;
this.status = "offline";
this._detachSocket();
this._detachParser();
this.root = null;
}

async _streamError(condition, children) {
Expand Down Expand Up @@ -184,6 +184,14 @@ class Connection extends EventEmitter {
this.emit(status, ...args);
}

_ready(resumed = false) {
if (resumed) {
this.status = "online";
} else {
this._status("online", this.jid);
}
}

async _end() {
let el;
try {
Expand Down Expand Up @@ -272,6 +280,7 @@ class Connection extends EventEmitter {
*/
async stop() {
const el = await this._end();
this.jid = null;
if (this.status !== "offline") this._status("offline", el);
return el;
}
Expand Down
4 changes: 1 addition & 3 deletions packages/connection/test/close.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import Connection from "../index.js";
import { EventEmitter, promise, timeout, TimeoutError } from "@xmpp/events";
import xml from "@xmpp/xml";
import { xml } from "@xmpp/test";

test("resets properties on socket close event", () => {
const conn = new Connection();
conn._attachSocket(new EventEmitter());
conn.jid = {};
conn.status = "online";
conn.socket.emit("connect");
conn.socket.emit("close");
expect(conn.jid).toBe(null);
expect(conn.status).toBe("disconnect");
});

Expand Down
10 changes: 10 additions & 0 deletions packages/connection/test/stop.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Connection from "../index.js";
import { JID } from "@xmpp/test";

test("resolves if socket property is undefined", async () => {
const conn = new Connection();
Expand Down Expand Up @@ -38,3 +39,12 @@ test("does not throw if connection is not established", async () => {
await conn.stop();
expect().pass();
});

test("resets jid", async () => {
const conn = new Connection();
conn.jid = new JID("foo@bar");

expect(conn.jid).not.toEqual(null);
await conn.stop();
expect(conn.jid).toEqual(null);
});
1 change: 1 addition & 0 deletions packages/resource-binding/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ async function bind(entity, iqCaller, resource) {
const result = await iqCaller.set(makeBindElement(resource));
const jid = result.getChildText("jid");
entity._jid(jid);
entity._ready(false);
return jid;
}

Expand Down
3 changes: 1 addition & 2 deletions packages/sasl-anonymous/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
"main": "index.js",
"keywords": [
"XMPP",
"sasl",
"anonymous"
"sasl"
],
"dependencies": {
"sasl-anonymous": "^0.1.0"
Expand Down
3 changes: 1 addition & 2 deletions packages/sasl-plain/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
"main": "index.js",
"keywords": [
"XMPP",
"sasl",
"plain"
"sasl"
],
"dependencies": {
"sasl-plain": "^0.1.0"
Expand Down
3 changes: 1 addition & 2 deletions packages/sasl-scram-sha-1/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
"main": "index.js",
"keywords": [
"XMPP",
"sasl",
"plain"
"sasl"
],
"dependencies": {
"sasl-scram-sha-1": "^1.3.0"
Expand Down
5 changes: 2 additions & 3 deletions packages/sasl/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,13 @@ test("with object credentials", async () => {
);

entity.mockInput(<success xmlns="urn:ietf:params:xml:ns:xmpp-sasl" />);

await promise(entity, "online");
});

test("with function credentials", async () => {
expect.assertions(2);

const mech = "PLAIN";
let promise_authenticate;

async function onAuthenticate(authenticate, mechanisms) {
expect(mechanisms).toEqual([mech]);
Expand Down Expand Up @@ -79,7 +78,7 @@ test("with function credentials", async () => {

entity.mockInput(<success xmlns="urn:ietf:params:xml:ns:xmpp-sasl" />);

await promise(entity, "online");
await promise_authenticate;
});

test("Mechanism not found", async () => {
Expand Down
8 changes: 4 additions & 4 deletions packages/sasl2/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,11 @@ async function authenticate({
}

// https://xmpp.org/extensions/xep-0388.html#success
// this is a bare JID, unless resource binding has occurred, in which case it is a full JID.
// this is a bare JID, unless resource binding or stream resumption has occurred, in which case it is a full JID.
const aid = element.getChildText("authorization-identifier");
if (aid) entity._jid(aid);
if (aid) {
entity._jid(aid);
}

for (const child of element.getChildElements()) {
const feature = features.get(child.getNS());
Expand Down Expand Up @@ -132,8 +134,6 @@ export default function sasl2({ streamFeatures, saslFactory }, onAuthenticate) {
}

await onAuthenticate(done, intersection);
// Not online yet, wait for next features
return true;
},
);

Expand Down
26 changes: 20 additions & 6 deletions packages/sasl2/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,17 @@ test("with object credentials", async () => {
</authenticate>,
);

entity.mockInput(<success xmlns="urn:xmpp:sasl:2" />);
entity.mockInput(<features xmlns="http://etherx.jabber.org/streams" />);
const jid = "username@localhost/example~Ln8YSSzsyK-b_3-vIFvOJNnE";

await promise(entity, "online");
expect(entity.jid?.toString()).not.toBe(jid);

entity.mockInput(
<success xmlns="urn:xmpp:sasl:2">
<authorization-identifier>{jid}</authorization-identifier>
</success>,
);

expect(entity.jid.toString()).toBe(jid);
});

test("with function credentials", async () => {
Expand Down Expand Up @@ -74,10 +81,17 @@ test("with function credentials", async () => {
</authenticate>,
);

entity.mockInput(<success xmlns="urn:xmpp:sasl:2" />);
entity.mockInput(<features xmlns="http://etherx.jabber.org/streams" />);
const jid = "username@localhost/example~Ln8YSSzsyK-b_3-vIFvOJNnE";

expect(entity.jid?.toString()).not.toBe(jid);

entity.mockInput(
<success xmlns="urn:xmpp:sasl:2">
<authorization-identifier>{jid}</authorization-identifier>
</success>,
);

await promise(entity, "online");
expect(entity.jid.toString()).toBe(jid);
});

test("failure", async () => {
Expand Down
Loading
Loading