Roster size restriction

I want to limit the roster size of users according to database requests. To be perfect, it should allow to put restriction based on subscription status (none, to, from, both), and on the domain part of the JID (to allow differences between chatrooms, users…). The reason behind this is : the “friend” status gives access to server-costly features, so we want to have strict control of it, on a per-user basis, and we don’t want to have 2 different notions of “friend”.

I see multiple ways to achieve this.

1 - Create my own RosterItemProvider (by extending DefaultRosterItemProvider). Throw an exception in the provider’s createItem, that should be caught in IQRosterHandler.java::handleIQ. As the code is now, it will send a internal_server_error back, unless I change existing code to add a new exception, and send a “forbidden” packet back when it’s caught.

2 - Use a plugin with a RosterEventListener, and give “addingContact” the possibility to forbid the new contact. The return value of addingContact is already used (to specify if the new roster item should be persisted by the Provider). So we either could use a new Exception for this case, or enrich the return value of addingContact to contain the 2 values. Both require changing existing code.

For now, the second solution seem better to me : it keeps Roster restrictions separate from Roster Provider. In my case, for example, the database containing the Rosters is the default one, but we need to connect to an other one to get the currently allowed Roster size. The first solution would imply to have 2 database connections in the RosterItemProvider. Also, the 2nd solution may be useful at some point for other plugins/core code : any RosterListener can forbid a contact being added to the Roster. But it means changing existing code to add facilities concerning only (for now) one single use case, so I would understand some reluctance.

Any advice is welcome

EDIT :

  • Pull request for small changes that allow code reuse from the plugin : Pull Request #753

  • Branch containing the plugin itself : Openfire/src/plugins/rosterRestrictions · GitHub

You might be able to do this without requiring a change in Openfire itself. Using a packetinterceptor, you could act on any relevant stanza that flows through Openfire. You could, for instance, intercept incoming roster modifications and/or presence subscription stanzas, and process them according to your business-rule.

Following your lead of a packetinterceptor, I’ve come up with some observations :

  • The RosterAdd requests are similar to RosterSet, so when a packet is intercepted, the interceptor has to check the JID to see if it’s a new one or an update, by calling the RosterProvider. I may be missing other edge cases here : I hope there’s a robust way to determine if a request will end up in a new entry in the Roster or not.

  • To have different limits for each SubscriptionType, the interceptor would have to intercept various kind of requests, and again reproduce the logic leading from a presence change to a RosterItem SubscriptionType change.

  • The interceptor has to access the RosterProvider, to fetch the current roster size anyways.

So I’m a bit torn :

  • on one hand the RosterEventListener is “closer” of the Roster business logic, so I think it will be easier to get it right.The plugin doesn’t have to reproduce the logic that leads from a request to an actual “add”.

  • on the other hand, interceptor can be coded right away. No need to change the Openfire’s Roster.java

I still believe that a RosterEventListener is a better way to do what I’m trying to do in the long run, but I’ll trust your decision concerning this issue. Do you still think that I should go on with the “interceptor” approach ?

I think I finally got why you were not fond my my way to implement “roster size restriction” :

Every time a roster item is modified, it’s done “in place”, and the RosterProvider or Listeners are called afterwards (ex : PresenceSubscribeHandler::manageSub, the call to updateState is done on the item directly). So even if a Provider or a Listener throws an exception, the memory version of the roster item will still have the changes applied.

I’m not certain about the caching policies in OpenFire, but I would not be surprised if that was an issue (like : people being forwarded presence changes because the memory item has subscribe=both, even the update has been denied/failed by the Provider or a Listener, until user’s next connection, or even until server’s next start).

The only way to go on with this approach would be to change all the callers of “Roster.updateRosterItem” (15 or so, mostly in plugins, only 2 in openfire’s core) to have them :

  • rollback the memory-item’s changes in case of failure,

  • or work on a copy of the object and have “Roster.updateRosterItem” itself write the memory-item after everything’s done correctly.

This is beginning to become a big structural change, so I bet it will never get merged if done this way :slight_smile: (very few people needing such features = very few testers, the only benefit being the ability to easily write plugins securely forbidding Roster changes in certain cases).

I’ll probably revert to my attempt using interceptors, good thing I kept the code

Thanks for your help, I’ll keep you updated.