Closed netmikey closed 2 years ago
You are correct - the resource binding iq shouldn't have that xmlns
.
To make sure that's the issue, can you give this a try:
// node_nodules/@xmpp/resource-binding/index.js
// replace function bind with
async function bind(entity, iqCaller, resource) {
const result = await entity.sendReceive(xml('iq', {type: 'set'}, makeBindElement(resource)));
const jid = result.getChild('bind', 'urn:ietf:params:xml:ns:xmpp-bind').getChildText("jid");
entity._jid(jid);
return jid;
}
Thanks a lot @sonnyp ! I can confirm that this solves that particular issue! :-)
@sonnyp Have you had the chance to implement a fix for this? If so, may I kindly ask for you to create a bugfix release containing it? Our project has been stuck on this for a while now ;-)
I had a look at making a bugfix for this recently and scratched my head for a while.
https://github.com/xmppjs/xmpp.js/issues/890#issuecomment-798961603 does not appear to solve the problem for me
You are using TCP correct? Could you share the exact patch / diff you're using ?
I've just been able to reproduce it again: doesn't work with current release of @xmpp/resource-binding
(v0.12.0, which is a couple of months old) and works after applying your 2-line fix:
node_modules/@xmpp/resource-binding/index.js
17,18c17,18
< const result = await iqCaller.set(makeBindElement(resource));
< const jid = result.getChildText("jid");
---
> const result = await entity.sendReceive(xml('iq', {type: 'set'}, makeBindElement(resource)));
> const jid = result.getChild('bind', 'urn:ietf:params:xml:ns:xmpp-bind').getChildText("jid");
Yes, since starttls is still having issues, I have disabled it in the server for now, and I'm using ANONYMOUS authentication:
this._client = client({
service: `xmpp://server:port`,
resource: 'my-client',
});
this._client.start();
** Edit: hmm, comparing the wirelog, it looks as if the only difference is the id
attribute on the iq
element, that seems what makes Openfire complain...
Here's what xmpp.js produces:
without the patch:
<iq type="set" id="0va04vxtta" xmlns="jabber:client"><bind xmlns="urn:ietf:params:xml:ns:xmpp-bind"><resource>my-client</resource></bind></iq>
with the patch:
<iq type="set" xmlns="jabber:client"><bind xmlns="urn:ietf:params:xml:ns:xmpp-bind"><resource>my-client</resource></bind></iq>
Could you undo the previous patch and try this instead
Replace node_modules/@xmpp/connection/index.js
send
method with
async send(element) {
const str = element.toString();
element.parent = this.root;
await this.write(str);
this.emit("send", element);
}
I'm confident it fixes your particular issue correctly but I'd like to make sure.
Hmm, are you sure about node_modules/@xmpp/connection/index.js
? Its send method looks quite a bit different:
async send(...elements) {
let fragment = "";
for (const element of elements) {
element.parent = this.root;
fragment += element.toString();
}
await this.write(fragment);
for (const element of elements) {
this.emit("send", element);
}
}
Replacing it doesn't change anything in the iq type set message, so doesn't seem to solve this particular issue.
Ok I'm confused again. Could you look into where the xmlns
on the resource binding iq is coming from ?
After comparing the messages that work with those that don't work in https://github.com/xmppjs/xmpp.js/issues/890#issuecomment-902134290, I don't think the xmlns
is the culprit anymore. The only difference left between the resource binding iq's that work and those who don't is the id
attribute. When it is set, openfire throws the error 401, and when no id
is set it works.
I think the issue is the combinaiton of xmlns
+ id
Psi worked didn't it?
Okay, I'm pretty sure I found it: it seems to be an escaping issue in xmpp.js.
I debugged this in Openfire and managed to track the issue down to this code:
Here, using the original unaltered xmpp.js code, the IQPacket
that comes into the method looks like this at runtime:
<iq type="set" id="b8vrmrxbud" from="my-server/1uykvio4g2" to="my-server">
<bind xmlns="urn:ietf:params:xml:ns:xmpp-bind"><resource>my-resource</resource></bind>
</iq>
What should be a child element is actually escaped and becomes a text node, so childElement
ends up being null
and the check for the binding case in line 104 is never reached.
When I alter the xmpp.js code, the child element isn't escaped, childElement
contains the right element and the check succeeds.
To check whether it's an issue on the xmpp.js client or the server, I disabled TLS, fired up wireshark and looked at what is sent over the wire, and sure enough, it's being sent in escaped form from xmpp.js:
I doubt that's it, I can't reproduce this and it would affect all servers, not just Openfire. I also had a quick glance at the code and I see no way this could ever happen (I could be wrong).
Are you sure your xmpp.js is untouched? To make sure rm -r node_modules/ && npm install
Here is my capture of bind
xmpp://localhost:5222
When I alter the xmpp.js code, the child element isn't escaped,
Could you expand on that?
Slightly off topic, I did find out why debug
reported that xmlns
was sent why wireshark shows it is not. Working on it.
We might be on to something, I just cleaned and re-run everything and can still confirm the escaped child element is being sent. My stack is:
Might be the outdated Node version on my side (wasn't aware of that actually, will try to upgrade this and report back).
* Edit: Nope I see the same behaviour with Node v14. Weird, why would it behave differently then?!*
When I alter the xmpp.js code, the child element isn't escaped,
Could you expand on that?
Oh yes, sure, I meant your workaround from https://github.com/xmppjs/xmpp.js/issues/890#issuecomment-798961603 - that makes it work, so it somehow makes the child element not escaped.
After what we found, I created a minimalistic project that only creates a client and connects to the Openfire server. This one doesn't escape the child element. So this clearly has to do with my project (I am replacing node-xmpp
with xmpp.js
in an existing project). So next step: figure out what in the world would make xmpp.js behave that weirdly in that project 😅
Not an xmpp.js bug.
It's embarrassing to admit, but for reference: it turns out I probably forgot to purge my package-lock.json
when I moved from node-xmpp to xmpp.js so the lockfile might have had some sticky dependency (maybe ltx
or something). Removing node_modules
and npm install
ing then re-installed the older dependencies sticked in the lockfile. Anyway, when I delete node_modules
AND package-lock.json
and redo everything fresh, it works as expected. Sorry for all that hassle 😅
I'm glad you were able to find the issue!
The mileading xmlns
printed by debug
is fixed in https://github.com/xmppjs/xmpp.js/pull/913
Describe the bug The connection with Openfire cannot be established because the resource binding is refused with a
not-authorized
message.Logs
Environment node v12.18.3 @xmpp/client v0.12.0 Server: Openfire 4.6.2, build b61bce3, configured with a valid TLS certificate for the domain I'm using. OS: macOS Catalina 10.15.7
Additional information This is a follow-up bug-report of #889 . I disabled starttls server side to be able to continue the connection attempt.
This issue might even be the hidden cause of #889.Turns out: it isn't.I've been debugging the handshakes and communication between xmpp.js and Openfire all day and I couldn't find a clear culprit. However, clients like Adium and Psi are able to connect to the same Openfire instance, so I guess it is something that the client library should handle.
I found that it doesn't make a difference whether a resource is specified by the client. I also found that it doesn't make a difference whether SASL type of ANONYMOUS or PLAIN is used (both pass the authentication but fail at the resource binding step).
When comparing a Psi connection setup vs. an xmpp.js one, the only notable difference was that Psi was not using the
jabber:client
namespace and was using a hardcoded id ofbind_1
(which, funny, is the one used in the XMPP specification's example) for the bindiq
message. So:Psi:
xmpp.js:
The specification also doesn't include the
jabber:client
namespace on the iq element, so that might be the issue. I've been unable to find a way to strip thatxmlns
off that singleiq
element though as it seems pretty heavily entangled within the library...Any help would be highly appreciated as I'm now completely stuck :(