Fix group member attribute selection for 389 Directory Server

[c1-dev-bot]

Summary

• Fix LDAP Object Class Violation (code 65) when granting/revoking group membership on 389 Directory Server
• The Grant and Revoke methods previously defaulted to uniqueMember (used by groupOfUniqueNames) for groups that didn't match posixGroup or ipausergroup/AD patterns
• 389 DS typically uses groupOfNames which requires the member attribute, not uniqueMember
• Restructured the object class switch to explicitly check for groupOfUniqueNames (the only class needing uniqueMember) and default to member for all other group types

Details

The root cause was in pkg/connector/group.go in both the Grant() and Revoke() methods. The switch statement logic was:

1. posixGroup → memberUid (correct)
2. ipausergroup or has objectGUID → member (correct but overly specific)
3. default → uniqueMember (wrong for groupOfNames, group, etc.)

The fix changes this to:

1. posixGroup → memberUid (unchanged)
2. groupOfUniqueNames → uniqueMember (explicitly targeted)
3. default → member (correct for groupOfNames, group, ipausergroup, AD groups, etc.)

This resolves the TODO comment that was already in the Revoke method acknowledging this needed to be addressed.

Test plan

• Verify grant/revoke works against 389 Directory Server with groupOfNames groups
• Verify grant/revoke still works against OpenLDAP with groupOfUniqueNames groups
• Verify grant/revoke still works against Active Directory with group object class
• Verify grant/revoke still works with posixGroup groups

Fixes: CXH-1451

---

Automated PR Notice

This PR was automatically created by c1-dev-bot as a potential implementation.

This code requires:

• Human review of the implementation approach
• Manual testing to verify correctness
• Approval from the appropriate team before merging

[linear-code]

CXH-1451 Add 389 Directory Server support to baton-ldap

[github-actions]

Claude finished @c1-dev-bot[bot]'s task in 1m 39s —— View job

---

PR Review: Fix group member attribute selection for 389 Directory Server

• Gather context and read changed files
• Analyze the diff against origin/main
• Provide code review feedback

---

Verdict: Approve — this is a clean, correct fix with sound reasoning.

What changed

The Grant() and Revoke() methods in pkg/connector/group.go restructured their switch logic for choosing which LDAP attribute to modify when adding/removing group members:

Object Class	Before	After
posixGroup	memberUid	memberUid (unchanged)
ipausergroup / has objectGUID	member	(removed — handled by default)
groupOfUniqueNames	(fell through to default)	uniqueMember (explicit case)
default	uniqueMember	member

Why this is correct

1. groupOfUniqueNames is the only standard LDAP objectClass that uses uniqueMember. Making it the explicit case and defaulting everything else to member is the right inversion. RFC 4519 (groupOfNames) and AD (group) both use member.

2. Removing the objectGUID proxy check is fine. The old code used objectGUID presence as a heuristic for Active Directory. AD's group objectClass uses the member attribute, which is now the default — so AD groups are handled correctly without the special case.

3. ipausergroup removal is safe. FreeIPA's ipausergroup also uses the member attribute, so the default case covers it.

4. Sync-side consistency is maintained. The Grants() method at line 276 already reads from all three attributes (uniqueMember, member, memberUid), so the read path handles any group type regardless of which attribute stores membership.

5. Grant and Revoke remain symmetrical — the switch logic is identical in both methods, which is important for correctness.

Minor issue

Const alignment (group.go:48): attrGroupDescription lost its column alignment with the other constants after attrGroupObjectGUID was removed. The original had two spaces before =; the new version has one. Running gofmt should fix this, but it's cosmetic and non-blocking.

// Current (misaligned):
attrGroupMemberPosix  = "memberUid"
attrGroupDescription = "description"  // ← one space short

// Expected:
attrGroupMemberPosix  = "memberUid"
attrGroupDescription  = "description"

Fix this →

Summary

The logic change is correct and well-targeted. The old default of uniqueMember was wrong for the majority of LDAP directory servers (389 DS, AD, FreeIPA), and the fix properly narrows uniqueMember to the one objectClass that actually requires it. The TODO in the Revoke method is appropriately resolved by this change.

[gromande]

I created another PR before the bot created this one: #160
My PR also includes a fix for LDAP account creation, which was also broken