Skip to content
This repository has been archived by the owner on Feb 14, 2019. It is now read-only.

Nested group members not picked up #205

Closed
acosonic opened this issue Jan 16, 2018 · 18 comments
Closed

Nested group members not picked up #205

acosonic opened this issue Jan 16, 2018 · 18 comments

Comments

@acosonic
Copy link

Expected behavior:

Having 1 group called Redmine Network
with members:
user1 - regular user
user2 - nested group
user3 - regular user

In redmine, should be created group Redmine Network (works)
user1 (works)
all users from user2 (works, but they are not in Redmine Network group), no group is assigned to them.
user3 (works)

I have tried all configurations, none works with nested groups.

@acosonic
Copy link
Author

For better understanding.
AD situation.pdf

@acosonic
Copy link
Author

I believe searching for users belonging to groups should be done the following way:

  1. Users are synced with Redmine
  2. Groups are synced
  3. Users belonging to groups are synced (by traversing down from group down to nested-groups)...

@thorin
Copy link
Owner

thorin commented Jan 16, 2018

I need to know your current configuration.

With nested groups enabled the plugin, starting on the user's direct groups, follows the parent groups and adds the user to all those groups.

This might not happen if for example you have a group search filter or a group base dn which does not capture the direct or parent groups.

@acosonic
Copy link
Author

Hm, then I guess only way is to use non dynamic groups. I haven't tried removing groups based dn.

@acosonic
Copy link
Author

Hm, we are talking about nested group.

If we are traversing whole AD tree, then it should pickup users.

If nested group is member of some Redmine group, then nested group's users should be picked up, and added to Redmine group...

@thorin
Copy link
Owner

thorin commented Jan 17, 2018

But when searching for the parent groups on ldap it uses the search filters and group base dn you configured on redmine.

The only way for it to work the way you are describing is for it to ignore the group base dn and group search filter when it is looking at the user immediate groups and when walking through its parent groups.

@acosonic
Copy link
Author

At this config (out of the box though) 2k12R2 windows, structure looks like this:
AD Group (B) that should be added/synced with Redmine group (A), where AD group (B) has attribute member, and users listed there should be added to redmine group (A). If some member is a group (nested AD group - C), then it's members should be added, to the same Redmine group (A)...

I believe sync should be reverse, first sync users, then sync groups (traversing the way I described)...

Ignoring filters should be ideal, or at last throwing some error if group cannot be accessed, but is listed in some group that should be synced with Redmine...

With large AD implementations, nested groups are necessary way to reduce errors and problems with permissions...

@acosonic
Copy link
Author

Actually ignoring groups based DN in this case would be ideal solution...

Because then groups can be nicely organised in OU's...

@acosonic
Copy link
Author

So group base DN is not ignored for first level, only for nested groups it's ignored, because nested group might belong to other OU...

@s-andy
Copy link

s-andy commented Jan 24, 2018

@thorin, please check a solution for this issue in this fork: #207

Shortly what was changed:

  • New option 'Nested groups base DN', which is used only for subgroups.
  • New option 'Create nested groups', which controls, whether subgroups are imported into Redmine.

@thorin
Copy link
Owner

thorin commented Jan 27, 2018

Thank you s-andy.
I left you a comment there. There's a modification which I don't understand.

@s-andy
Copy link

s-andy commented Feb 6, 2018

@thorin, hm, I don't see any comments.

@thorin
Copy link
Owner

thorin commented Feb 7, 2018

Sorry @s-andy I expected you to be able to see it on your PR #207.

Basically, on the following lines:

@@ -164,7 +164,13 @@ def sync_user_groups(user)
         end
 
         changes = groups_changes(user)
-        added = changes[:added].map {|g| find_or_create_group(g).first }.compact
+        added = changes[:added].map {|g|
+          if setting.create_nested_groups?
+            find_or_create_group(g).first
+          else
+            ::Group.where("LOWER(lastname) = ?", g.mb_chars.downcase).first
+          end
+        }.compact

... you now only create new groups when the create_nested_groups flag is enabled.

Am I correct? Was this the intended behaviour?
That would break what we currently have for users which migth have that option disabled.

@s-andy
Copy link

s-andy commented Feb 9, 2018

@thorin, yes, it was intended. This is for the cases, when we have a more complicated group structure in AD and don't want it to be imported to Redmine (users are added to parent groups only).

Yes, you're right. It's probably better to change that option to something like dont_create_nested_groups.

@acosonic
Copy link
Author

acosonic commented Feb 9, 2018

Please keep in mind that nested groups might be in completely different base_dn search...

And Redmine admins don't want nested groups created, but they want users instead, assigned to parent groups (the ones which do get created).

FYI, we were able to achieve that with Andy's modification.

And user search filter which is built-in in Redmine, but it is a bit hard to edit on large AD's though.
too small textbox.

@thorin
Copy link
Owner

thorin commented Feb 9, 2018

With that change, new groups will be created if you call directly the rake task redmine:plugins:ldap_sync:sync_groups.
But they won't be created when you run redmine:plugins:ldap_sync:sync_users or a user logs in on redmine, unless you enable the create_nested_groups flag.

I would guess that's a undesirable effect for some and possible many users of this plugin.

@acosonic
Copy link
Author

acosonic commented Feb 9, 2018

If I do create nested groups, then I would have lot's of junk groups inside Redmine.

@thorin
Copy link
Owner

thorin commented Feb 9, 2018

Not everyone runs the sync_groups task.
With this change they won't have any new ldap groups being created at all.
I can't merge it like that.

@thorin thorin closed this as completed Feb 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants