-
Notifications
You must be signed in to change notification settings - Fork 429
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
Support dynamic domains in mod_roster #3159
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3159 +/- ##
==========================================
+ Coverage 79.56% 80.30% +0.74%
==========================================
Files 396 396
Lines 32287 32260 -27
==========================================
+ Hits 25688 25908 +220
+ Misses 6599 6352 -247
Continue to review full report at Codecov.
|
cba74c2
to
8a08909
Compare
Return "internal server error" for all IQ requests, not only for roster.
This is against RFC 6121 for roster set. For roster get it is unspecified, but very unexpected as well. Following commits will change the tests and the implementation.
According to RFC6121, section 2.3.3, it is forbidden to send roster set to another user's JID. In the current implementation there was only a check if the domain exists, which would leak information about other tenants served by the system. Handling of roster get to another user's JID is unspecified by the RFC, but it makes most sense to handle it the same way - it is better to return an error than to accept such stanzas, which are most likely sent by accident.
d3ddd91
to
d85befc
Compare
For 'rosterusers' it was there but it was unused. Two more unused columns are removed: 'subscribe' and 'type'.
The 'raw_to_record' callback was a no-op for all backends.
Rework the backend callbacks: - Add the host type - Merge all 'get_roster_entry_*' variants into one function - Remove unused 'raw_to_record' - Define types for cleaner and shorter type specs Make 'set_roster_item_t' return an error if a non-existing item is removed. This allows to reduce the extra check before calling 'set_roster_item'.
d85befc
to
765da85
Compare
- Update callbacks - Update prepared queries
Take it from the creds
The calls are preceded with 'does_user_exist', so we can assume the host type exists.
ets:lookup_element/2 was throwing 'badarg' for empty feature lists
Dynamic domains don't support BOSH and these test cases shouldn't depend on BOSH anyway.
765da85
to
3e5cd0e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Main changes:
mod_roster
to use host types.Side changes:
mod_roster
. They started becoming tedious to maintain.mod_roster
to make thetest type
section consistent with the top-level ones by excluding the header from the template. This makes the templating itself a bit inconsistent, but IMO other modules can be converted as well. It makes sense to exclude constant parts from the template.Not changed:
roster_groups
hook is only used bymod_pubsub
and has no handlers. It can be removed, but we should figure out whether it needs a fix, as the resulting config field is always an empty list.Take a quick look at the commit messages for more information.