Bug 133861

Created attachment 104407 [details]
preleminary version of a patch addressing the issues.

The patch addresses some of the issues.
1. the ssl connection will be established correctly.
2. when new entry is added, code adds most common objectClass definitions.
3. .. and chooses to call ldap_modify or ldap_add apriopriately.

There are some unadressed issues related to the fact that commonly used LDAP
schemas do not have shadow attributes - aporpriate options should probably be
added to libuser.conf. Also, two attribues in libuser library are mapped to
single userPassword entry which leads to failed add calls: only workaround is
implemented.

Suggestions and comments are welcome.

Have you took a look if this still applies against FC3 rawhide? 
Things may have changed much since FC2.

I see I should have done that - I looked at the 0.5.12 code and it
seems 2 and 3 are addressed there (Addmitedly with a number of new
"FIXME"s and "Ugly hack"s)! I will try to port what's left of my patch
to the new version.

Created attachment 104440 [details]
libuser-ldap.patch

Next version of the patch, generated against 0.51.12.
- add "shadow" configuration keyword to disable generation of shadow-type keys
which would lead to schema violations on servers that do not have the shadow
schema enabled.
- use ldap_initialize() to allow LDAP over SSL or IPC.
- provide more detailed information on bind failures.
- add created group entries to posixGroup class (schema violation otherwise).
- luseradd: use lu_strerror() to provide more information on failures.

Thanks a lot for your patches.
T
he first libuser version that should actually be usable for LDAP is
0.52, which already contains some equivalent changes.

- LU_LDAP_DOSHADOW:
  Is there any real reason not to simply enable the shadow schema on
  the server? It should't hurt anything, and I have noticed that
  system-config-users doesn't like minimal posixAccount entries much.
- SSL: Nice patch, this way it is actually possible not to use TLS
  if you specify ldap://....
- Removing simple binding using generated bind DN,
  ldap_err2string(): It is not quite clear what bind attempt does the
  error message refer to; we should probably report the first error
  instead of the last.
  Anyway, we are string frozen for release now, so this will have
  to be postponed post-FC3.
- ent_name: 0.52 has s/User/Entity/
- ent_class: 0.52 has similar code, your variant ignores
  LU_LDAP_SHADOW
- Adding posixGroup explicitly should not be needed, LU_GROUPNAME
  pulls posixGroup in.  Can you reproduce the schema violations
  with 0.52.*?
- the `type' argument to get_ent_adds () is not used anywhere
- luseradd changes: again blocked by string freeze.

0.52.2, which contains the ldap_initialize() change, should appear
in rawhide soon.

I'll leave this bug open to track the error message enhancements.
Thanks again.

re: shadow: I think it can be useful  particularly if the LDAP server
is not controlled by the libuser's user, ie. the one organizational
entity manages the user database used by another group. I do not know
how common it is.

re: removing binding with generated bind DN: since bind DN is
configurable, there is no reason to override the user choice and try
other bind DN. With the proposed change, the returned error message
allows to establish why bind failed (there might be other reasons than
just wrong password).

Regarding the remaining comments, I will try 0.52 and report back.

libuser-0.52 seems to work just fine (apart from cryptic messages on
bind failures). As it turns out, our depertamental server supports
shadow schema as long as "shadowAccount" class is used.

libuser-0.52.6-1, to appear in rawhide probably after FC3 release,
should contain more verbose and consistent error messages, including
the luseradd changes from your patch.

The generated bind DN code was left in place (because it can guess
right very often), the error messages relate to the non-generated
bind DN and spell it out explicitly.

Thanks again.