Support for Pre-Approving a Subscription Request
Add support for 3.4. Pre-Approving a Subscription Request
#1590 add support for subscription pre-approval in RosterModule and RosterItem
#6 Updated by Daniele Ricci almost 4 years ago
I'd like to contribute to this feature. What approach do you suggest?
I can see that a subscription transition state is 'none' if coming from 'none':
Is that right? Could it be enough to forcibly change it to none_pending_out? I'm trying to figure out how it works.
#8 Updated by Daniele Ricci almost 4 years ago
This might be a problem:
3. If the contact is in the user's roster with a state of "To", "None", or "None + Pending Out", the user's server MUST note the subscription pre-approval by setting the 'approved' flag to a value of "true", then push the modified roster item to all of the user's interested resources. However, the user's server MUST NOT route the presence stanza of type "subscribed" to the contact.
Because Tigase roster doesn't support a state of pre-approval (that is, something like the 'approved' flag). Do we need a new subscription state for this?
#10 Updated by Wojciech Kapcia almost 4 years ago
I'd say that new subscription state is not needed and most likely inclusion of checking of
approved flag in
tigase.xmpp.impl.roster.RosterAbstract.StateTransition.getStateTransition() for transition be enough in terms of transition. If that wouldn't be enough then new set of transition states for "To", "None", or "None + Pending Out" would be needed.
#11 Updated by Daniele Ricci almost 4 years ago
I'm afraid using a state transition isn't enough either.
Pre-approval needs "to read the other roster" (that is, from the other contact involved in the subscription flow): presence subscription stanzas (type=subscribe(d)) are kept in storage and not delivered until the other contact comes online - therefore not setting subscription=from in the roster until the type=subscribe is delivered.
#12 Updated by Wojciech Kapcia over 3 years ago
Daniele Ricci wrote:
Pre-approval needs "to read the other roster" (that is, from the other contact involved in the subscription flow)
How come? This shouldn't be the case and contacts may not be on the same server/installation. And you can extend Roster item to hold any additional data.
#20 Updated by Daniele Ricci almost 3 years ago
Just to let you know that I began working on this on my fork .
All I have left is to reply with a subscribed stanza on behalf of the user when the contact requests a subscribe (and remember to not forward that subscribe). It should be all done by tomorrow. In the meantime, you're welcome to inspect the code to check if I did anything stupid so far :)
#21 Updated by Daniele Ricci almost 3 years ago
I finished! I tested it a bit and it seems to be working.
The only thing I couldn't workaround is that when the user sends "subscribe" to a contact that has pre-approved him before sending his initial presence, the incoming pre-approval is not processed yet at that point (state transition hasn't happened yet) so an automatic "unsubscribe" is triggered which causes the incoming pre-approval to be lost (because of the next state transition). This is in contradiction with RFC 6121, par. 4.3.2:
If the contact account does not exist or the user's bare JID is in the contact's roster with a subscription state other than "From", "From + Pending Out", or "Both" (as explained under Appendix A), then the contact's server SHOULD return a presence stanza of type "unsubscribed" in response to the presence probe (this will trigger a protocol flow for canceling the user's subscription to the contact as described under Section 3.2; however, this MUST NOT result in cancellation of a subscription pre-approval as described under Section 3.4).
However, I think sending subscription requests before sending an initial presence is a bit unusual (if not even allowed because you're not an active resource yet, what do you think?).
The URL to access the patch is the same, if you give me the OK I will send you an official patch on top of latest release branch (the code I've been working on is based on commit 81088be7c18e7f1df04f7020301b9c9708d4a008).
#22 Updated by Daniele Ricci almost 3 years ago
I've just rebased my repository on latest release branch:
As you might already know, you can append .diff or .patch to access a unified diff or a git patch:
Looking forward to your review :)
#29 Updated by Daniele Ricci over 1 year ago
For the record, this is the correct patch (the 6 commits Wojciech is talking about):
Thanks for reviewing! If you need anything just ask.
#34 Updated by Wojciech Kapcia over 1 year ago
- Due date changed from 2017-05-23 to 2017-07-13
- Status changed from In Progress to In QA
- % Done changed from 0 to 90
I've finished with the implementation (after the review of provided patch, which required changes); added TTS-NG test case as well as modified JaXMPP to match.
@Daniele Ricci, a couple of comments:
there wasn't a need for distinct approved_in and approved_out as it's only based on user roster (not contact, i.e. it's local)
outgoing subscribed presence wasn't filtered out;
"subscribed" presence was not generated automatically.
#35 Updated by Daniele Ricci over 1 year ago
Wojciech Kapcia wrote:
- there wasn't a need for distinct approved_in and approved_out as it's only based on user roster (not contact, i.e. it's local)
Oh. Ok, thanks.
- outgoing subscribed presence wasn't filtered out;
- "subscribed" presence was not generated automatically.
Mmm... it is working on production and it does generate a subscribed.
Anyway, I'll backport this to 7.1.1-SNAPSHOT and test it as soon as possible.