Opened 4 years ago

Closed 8 months ago

# Please support specifying full DN of group whose membership is required to log in

Reported by: Owned by: Andras Korn Andras Korn normal DirectoryAuthPlugin normal

### Description

It is sometimes desirable to have LDAP groups used for trac access control in a separate subtree from groups used for e.g. role management (such as granting TRAC_ADMIN privileges).

Currently, the plugin doesn't support specifying group DNs anywhere; all groups are only referred to using their cn, which is not guaranteed to be unique if the search can span several subtrees.

In 14830:

refs #12068
refs #11686
refs #11361
refs #11307
refs #11304
refs #10878
refs #10715
refs #10667
refs #10632
refs #10631
refs #11015

various fixes

### comment:2 follow-up:  3 Changed 2 years ago by bebbo

Resolution: → fixed new → closed

the group search base dn ist configured via group_basedn

### comment:3 in reply to:  2 Changed 2 years ago by Andras Korn

Resolution: fixed closed → reopened

the group search base dn ist configured via group_basedn

Unfortunately that's insufficient. As explained in the Description, there may be several groups with the same cn under group_basedn; that's why it is necessary to be able to select a specific group by its full DN.

### comment:4 Changed 2 years ago by branson

So that's not really possible. Groups are expanded into the Trac "namespace" by their CN. What you're suggesting has a few problems:

• the nomenclature is "@" + {groupname} .. so "@admin". You can't use @{DN} as many DN's have spaces and that won't parse.
• If you have multiple CN's with the same name under group_basedn .. with different permission models.. you'll have trouble. The search is going to return the first entry and use that on the lookup, and it's rather non-deterministic as to which one will come first. That's why we use a group_basedn to identify the top of the tree where we know it'd be right.

If you have this case.. i'd suggest you create a sub OU under group called Trac .. and then define your permissions there, unique to the definitions.

You could also explore other ways to resolve this:

1. allow more than one group_basedn to be defined, and the ordering of the definition would specify precedence. While this might work it's a penalty in search.. and will be complex.

### comment:5 follow-up:  6 Changed 2 years ago by bebbo

btw: I dropped the '@' prefix since it caused trouble elsewhere... and I did not see any benefit of this prefix.

And I agree that a structured LDAP tree should have all trac groups as children/grandchildren/etc. under one ou node.

Thus specifying this sinlge ou as the group_basedn and with all permission groups having unique names there is no way for any duplicate.

### comment:6 in reply to:  5 Changed 2 years ago by branson

btw: I dropped the '@' prefix since it caused trouble elsewhere... and I did not see any benefit of this prefix.

You might want to write a db update to migrate the permissions models for adopters that are using the current implementation. The use of '@' was the original configuration before I took on this plugin before; I left it that way as there were usernames that matched group names in AD... and there were also situations where local 'group' names may have been the same as AD group names (anonymous) .. and this was a good way to make sure the groups were uniquely identified.

Also helped identify the groups from the users... in my the last few implementations we had ~1500 users and ~300 groups. Doesn't hurt to separate it .. but will limit the implementations for some people and won't be backwards compatible unless you update the database as well.

### comment:7 Changed 2 years ago by bebbo

In 14836:

refs #11307

readded the '@' prefix for ldap groups

### comment:8 Changed 8 months ago by bebbo

In 16071:

refs #11307

added a new config property 'group_nameattr'. set this to 'dn' and then use the distinguished names to specify the user / admin group.

E.g.:

group_nameattr = dn
group_validusers = @cn=user,ou=trac,ou=groups,dc=foo,dc=bar

(Note: group_tracadmin is not used -> misleading -> removed

Last edited 8 months ago by bebbo (previous) (diff)

### comment:9 Changed 8 months ago by anonymous

please test with the version 2.1.0-SNAPSHOT

### comment:10 Changed 8 months ago by bebbo

In 16072:

refs #11307

fixed handling to use the configured attribute instead of 'cn'

### comment:11 Changed 8 months ago by bebbo

Owner: changed from branson to Andras Korn reopened → assigned

### comment:12 Changed 8 months ago by Andras Korn

Thanks for working on this!

Unfortunately, my original use case is no longer relevant, so it'll be a while before I can test the new functionality.

### comment:13 Changed 8 months ago by bebbo

Resolution: → fixed assigned → closed

my tests with 'dn' instead of 'cn' were successful

### Modify Ticket

Change Properties