Openfire

Having this would be great. In the meantime I tried to automate this with something along the lines of:

GroupManager groupManager = GroupManager.getInstance();

ArrayList<Group> groups = new ArrayList<Group>(groupManager.getGroups());

for (Group group : groups) {
group.getProperties().put("sharedRoster.showInRoster", "onlygroup");
group.getProperties().put("sharedRoster.displayName", group.getDescription());
group.getProperties().put("sharedRoster.groupList", toList(new String[] {}, "UTF-8"));
}

but although this code appears to be correct, and viewing a group in the web interface shows these changes have been made, the groups are not pushed to clients unless I click "save changes" on the group-edit.jsp (without having made any changes), at which point the groups appear in connected clients. Is this just a roster caching issue or have I misunderstood/got something wrong?

Phil,
Where exactly are you inserting that code?

I'm attaching a proof-of-concept patch for anyone who wants to hardwire the autosharing. The patch simply changes two lines of code in RosterManager.java. After applying the patch, you should be able to create a system property called "groups.autoshare" and set it to either "onlyGroup" or "everyone". I'm not claiming this to be the best solution...it's really more of a hack, but it gets the job done.

Disclaimer: I haven't actually compiled this code since I don't have a java dev environment to test it with. I'm hoping it just gets blindly pushed into the trunk, so I can test it in 3.3.3. ;-p

For this to work don't you also have to have set the groups to be shared in the first place? The collection you iterate over is

Collection<Group> groups = GroupManager.getInstance().getSharedGroups();

so this is only part of the solution. If you want to make sure that all groups are automatically shared it should be changed to

Collection<Group> groups = GroupManager.getInstance().getGroups();

Is that right?

btw I was inserting that code after containerClass.newInstance(); in org.jivesoftware.openfire.starter.ServerStarter.java (also tried as a standalone class which instantiated an XMPPServer class - both seemed to get me to the same state as mentioned before)

Phil,
Thanks for the feedback. I've attached a new diff file to use getGroups() and also noticed that I needed to update getPublicSharedGroups() just below the getSharedGroups() function I was tinkering in.

Again, I'm not compiling this code since I don't have a dev environment – I'm just trying to spur some activity on this issue.

Geez... Uploaded another diff. I forgot to use getGroups() from within getPublicSharedGroups(). Sorry for the noise.

Finally got to look at this again. Your latest patch does build but it doesn't work for me. I'll put some debugging in to see what's going on and what groups are being returned.

oops should have looked at the patch more closely- string equality needs changing to .equals() not ==

this appears to work but populateGroups() in LDAPGroupProvider is basically killing me now. About thirty seconds per-group, over a hundred groups = not fun.

Sorry about the .equals() bug – I'm not a real programmer.

Is populateGroups() killing you because of this patch or is it just the fact that you have so many users & groups? Put another way: if you manually shared all of your groups, would populateGroups() be just as slow?

Okay, I've attached a new version of the patch that fixes the string comparisons.

OK, finally looked at this again. After applying your patch the group won't be sent to the client unless two group properties are set.

I put

group.getProperties().put("sharedRoster.showInRoster", "onlyGroup");
group.getProperties().put("sharedRoster.displayName", group.getDescription());

just before answer.add(group); in the onlyGroup "if" code block.

That seems to do the trick. I don't use any global groups but presumably this would need duplicating for the global groups "else if". I'd provide a diff but I've modified other parts of this code to add logging etc.

Phil, thanks for the feedback.

I finally had some time today to setup a dev environment here at the office to test the patch. After fixing some stupid mistakes ("everybody" not "everyone"!) and do some more testing, I think the patch I'm uploading now ready for submission to the Jive QA team.

To correct my previous comments: in order to enable this feature, you must create a system property called "groups.autoshare" and set the value to either "onlyGroup" or "everybody".

Phil, please test the patch if you can. If everything goes well for you, I'll try to get Gato or Matt to send it to their QA folks for possible inclusion in the 3.4 release.

Thanks

Cameron, that's excellent - the code looks good to the eye but I'm away for a week now and won't be able to check it until I get back.

Guess what? I'm uploaded one last version of the patch. I started thinking that it would suck to enable sharing to everybody and then decide that that kind of roster didn't work well in your organization. With my previous patches, you'd have to go unshare everything manually. Yikes! But with this latest patch, you can now set "groups.autoshare=nobody" and have all the sharing removed automagically. Yeah!

Here are a few effects of this patch:

1) Since we alter the global RosterManager, it works for all group providers, not just LDAP.

2) The initial roster population and autosharing is considerably slower than I expected when set to "everybody", but once the JIVEGROUPPROP table is updated, non-cached roster builds go pretty quick. Testing locally on my 2.8GHz workstation with 150+ LDAP users in 21 LDAP groups using the embedded-db, I got the following times:
nobody->everybody = 30sec
everybody->nobody = 8sec
nobody->onlyGroup = 1sec
everybody->onlyGroup = 3sec

3) Once the autosharing happens, the sharing of those groups is persistent in the DB even if you delete the autoshare system property. To remove the sharing, set "groups.autoshare=nobody".

4) Once enabled, all groups will be reshared using the new autoshare policy regardless of what they were set to previously. This patch modifies existing groups as well as new ones. For example, if you have all your groups manually set to share to "onlyGroup" and you set "group.autoshare=everybody", all existing groups will be reshared to "everybody", effectively overwriting the old showInRoster values.

As far as I can tell, this all works. Any chance of it appearing in 3.3.4?

What will the default value be for this? I hope it will be disabled by default. We have a 30,000 user directory, with hundreds of LDAP Groups. So, if this is enabled be default, people with a large number of groups could have a problem.

Aaron,
It's disabled by default. Openfire should behave as it normally does until the "groups.autoshare" system property is defined.

Phil,
I asked Gato about this patch in the Sept. 12th Dev chat. He said he hasn't had time to look it over yet. Feel free to pester him about it.

Hi,
i tried the patch on openfire 3.3.3 with ldap integration but it doesn't work !
The description of the groups are not automatically set...
Any solution ? This fonctionnality is very important in large organisation with ldap integration.
Thanks
Glocal [Minefe]

Hi all,

I tried another approach to implementing this, posted it on the forum here :
http://www.igniterealtime.org/community/thread/30462

The patch is somewhat simpler, and does not offer any configuration options for the moment, but it does allow to override group sharing settings for groups from LDAP. What do you guys think?

Is there any chance of this reaching next release, in this form or another? Is there anything I can do to help things along?

Well, Jonathan, you're patch is definately simple – I'll give you that; but I think it's solving a different problem specific to LDAP. The patch Phil and I have proposed should work with any Group Provider, not just LDAP. I didn't think to check what would happen if you shared a new LDAP group, so you're patch may still be necessary.

Cameron,

Yup, just LDAP - as stated by the issue's subject I realize this is a desirable feature for all Group Providers, but it is clearly most necessary with LDAP - imagine importing 500+ groups from LDAP and having to click through and share each one!

While there's lots of room for improvement, I was wondering if a small patch like this could be integrated into a minor release soon? I was really hoping to get this issue moving a bit, even if just through a small correction like this one for now, and working on a more generic one later? As I said, I have time and motivation to work on this.

Jonathan,

I was in the same position a few months ago and worked as hard as I could get the patch working and tested as best I could. I thought it was a relatively straight-forward patch and simple to follow and test, but I've not heard anything from Jive. I've since moved on to other projects here at work and have not had time to keep poking the developers to take a serious look at the patch.

Have you tried our patch? Does it work for you? Did you have any problems with it? If you have time, get busy on the forums and in the dev chat to draw some more attention to this issue. Thanks

Cameron,

I have tried your patch, of course - my alternative was only to try and speed things up with something simpler. However, one of the limitations of your patch is that once the group.autoshare property is set, all groups always have that parameter for sharing. I can see many a situation where I need all groups shared by default, but have to override some, and keep this persistent. Maybe group properties should only be set if they don't already have a value?

Thanks for your feedback on the history of this issue. I feel some guidance may be needed from developers here. I have posted two messages related to this on the forum, but will swing by the dev chat soon.

Jonathan,

Thanks for the feedback. I hadn't taken into consideration the fact that you may not want all existing groups shared. A couple thoughts:

1) Once you turn it on and have it set to only share unshared groups, you couldn't revert the sharing.

2) Perhaps we could use a "niceness" flag. Example: groups.autoshare_nice="true" would mean "only shared groups that haven't already been shared". groups.autoshare_nice="false" would mean "set all groups to share exactly the same, overriding any existing values." Insert a few if-statements into my patch, and we're good.

My naming of the property is debatable. Maybe "autoshare_force" would make more sense. Either way, you get the idea.

One other thing I would like to see eventually would be a way to set these parameters from the admin interface instead of having to manually add system properties. If you're bored, feel free to take a stab at that.

Thanks for your work, Jonathan. It's good to see someone else interested in this feature.