Bug 8993

Summary: LDAP authentication improvements: multiple branch and transformation modules
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: AuthenticationAssignee: Bugs List <koha-bugs>
Status: Failed QA --- QA Contact:
Severity: enhancement    
Priority: P5 - low CC: amitddng135, astronomypt, charles.farmer, ctodd, dcook, dpavlin, elliott, gaetan.boisson, gitbot, katrin.fischer, magnus, marjorie.barry-vila, martin.renvoize, mtompset, philippe.blouin, thibault.lemeur
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31378
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 8993: LDAP authentication improvements
Bug 8993: Sample LDAP transformation module
Bug 8993: LDAP authentication improvements
Bug 8993: Sample LDAP transformation module
Bug 8993: Follow-up - LDAP authentication improvements
Bug 8993: LDAP authentication improvements
Bug 8993: Follow-up - LDAP authentication improvements
Bug 8993 - my attempt to get this module working
Bug 8993 [Follow-up] update and replicate options used at wrong place
Bug 8993: LDAP authentication improvements
Bug 8993: Follow-up - LDAP authentication improvements
Bug 8993 - my attempt to get this module working
Bug 8993 [Follow-up] update and replicate options used at wrong place
Bug 8993: Add branchcode and categorycode mappings for sample module
Bug 8993: LDAP authentication improvements
Bug 8993: Follow-up - LDAP authentication improvements
Bug 8993 - my attempt to get this module working
Bug 8993 [Follow-up] update and replicate options used at wrong place
Bug 8993: Add branchcode and categorycode mappings for sample module
Bug 8993: LDAP authentication improvements
Bug 8993: Follow-up - LDAP authentication improvements
Bug 8993 - my attempt to get this module working
Bug 8993 [Follow-up] update and replicate options used at wrong place
Bug 8993: Add branchcode and categorycode mappings for sample module
Bug 8993: LDAP authentication improvements
Bug 8993: Follow-up - LDAP authentication improvements
Bug 8993 - my attempt to get this module working
Bug 8993 [Follow-up] update and replicate options used at wrong place
Bug 8993: Add branchcode and categorycode mappings for sample module
Bug 8993: LDAP authentication improvements
[DO NOT PUSH] Bug 8993: Sample LDAP transformation module
Bug 8993: LDAP authentication improvements
[DO NOT PUSH] Bug 8993: Sample LDAP transformation module

Description Julian Maurice 2012-10-31 10:26:34 UTC

    
Comment 1 Julian Maurice 2012-10-31 10:28:07 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2012-10-31 10:30:53 UTC Comment hidden (obsolete)
Comment 3 Julian Maurice 2012-10-31 10:32:17 UTC
Contents of POD file:

LDAP auth_method configuration
    This document is a step by step explaination for the new LDAP
    configuration method. The old one (see C4::Auth_with_ldap) still exists
    and you should give it a try if your needs of mappings and
    transformations are low.

  What do I have to ask to the directory administrator
   How to reach the service
    What we need is build the url of the ldap service we want to reach. So
    ask him for the URL. If he don't know, ask for the scheme (or protocol),
    the hostname and the port of the directory. Only hostname and scheme are
    mandatory.

    The scheme must be ldap or ldaps (ldaps is for crypted ldap, ldap over
    SSL). The url construction is:

        scheme://hostname:port
        scheme://hostname

    examples

        host=directory.example.com port=389 scheme=ldap

    gives you:

        ldap://directory.example.com:389

    Also: Active Directory (the Microsoft implementation) is an LDAP alike
    directory but Koha have to know it's active directory to use it, so
    please ask.

   What is the binding method ?
    Are anonymous allowed to make some searches on the server? If not, what
    is the credentials for the koha account ? Credentials are a pair (DN,
    password)

        uid=koha,ou=people,dc=example,dc=com MYS3CRET

    in Active Directory, it can be a login@example.com with a password

        koha@example.com MYS3CRET

   Now configure koha
    This is a basic exemple of things you will add in the "/config" part of
    "koha-conf.xml".

    First of all, you need to tell koha that ldap authentication is now
    relevant

        <useldapserver>1</useldapserver>

    If you use anonymous method, you have to give credentials of koha
    account

        <ldapserver url="ldap://directory.example.com"
              manager="uid=koha,ou=people,dc=example,dc=com"
              password="MYS3CRET"
              authmethod="search_dn" >

    Also, you can set the values for replication and update (documented in
    legacy pod):

        <ldapserver url="ldap://directory.example.com"
              manager="uid=koha,ou=people,dc=example,dc=com"
              password="MYS3CRET"
              authmethod="search_dn"
              replicate="1" update="1"
        >

    You must now tell koha the ldap branches where to find users

        <useldapserver>1</useldapserver>
        <ldapserver url="ldap://directory.example.com"
              manager="uid=koha,ou=people,dc=example,dc=com"
              password="MYS3CRET"
              authmethod="search_dn"
              replicate="1" update="1"
        >
          <branch dn="ou=people,dc=site1,dc=example,dc=com" />
          <branch dn="ou=people,dc=site2,dc=example,dc=com" />
          <branch dn="ou=people,dc=site4,dc=example,dc=com" />

    The best part of this new config is that you don't have to rely on
    simple mappings anymore (but you still can): you can use a perl module
    to write much sofisticated transformations directly in perl:

          <transformation module="C4::LDAPTransform::CustomTransform" />

    This module only have to define a subroutine named get_borrower
Comment 4 Julian Maurice 2012-10-31 11:21:40 UTC Comment hidden (obsolete)
Comment 5 Julian Maurice 2012-10-31 11:22:08 UTC Comment hidden (obsolete)
Comment 6 Alex Arnaud 2012-12-21 10:39:31 UTC Comment hidden (obsolete)
Comment 7 Chris Cormack 2012-12-24 08:47:49 UTC
The content of attachment 13130 [details] has been deleted by
    Chris Cormack <chris@bigballofwax.co.nz>
who provided the following reason:

Asked for by Julian

The token used to delete this attachment was generated at 2012-12-24 21:47:36 NZDT.
Comment 8 Chris Cormack 2012-12-24 08:48:49 UTC
The content of attachment 13132 [details] has been deleted by
    Chris Cormack <chris@bigballofwax.co.nz>
who provided the following reason:

Asked for by Julian

The token used to delete this attachment was generated at 2012-12-24 21:48:42 NZDT.
Comment 9 Julian Maurice 2013-01-08 11:00:08 UTC Comment hidden (obsolete)
Comment 10 Julian Maurice 2013-01-08 11:00:27 UTC Comment hidden (obsolete)
Comment 11 Julian Maurice 2013-01-08 11:01:34 UTC
Patches rebased on master.
Removed 2 scripts from the first patch that were not intended to be included.
Comment 12 Chris Cormack 2013-01-10 17:38:45 UTC
The content of attachment 13129 [details] has been deleted by
    Chris Cormack <chris@bigballofwax.co.nz>
without providing any reason.

The token used to delete this attachment was generated at 2013-01-11 06:38:38 NZDT.
Comment 13 Chris Cormack 2013-01-10 17:40:14 UTC
The content of attachment 13131 [details] has been deleted by
    Chris Cormack <chris@bigballofwax.co.nz>
without providing any reason.

The token used to delete this attachment was generated at 2013-01-11 06:39:27 NZDT.
Comment 14 Thibault Le Meur 2013-01-17 14:30:43 UTC
The patch is applied in our production environment which has several LDAP branches on a single LDAP server.
It is fully working.
Comment 15 Gaetan Boisson 2013-01-28 15:17:38 UTC
Patch tested on an instance with a "regular" single branch LDAP : everything working as expected, no regression!
Comment 16 Elliott Davis 2013-03-05 19:54:06 UTC
Sorry fir butting in here but I am confused.  Is this intended to be pushed or just for a customer?

Elliott
Comment 17 Julian Maurice 2013-03-11 08:26:43 UTC
Hi Elliott,

this patch is not customer-specific and is intended to be integrated into master.
Comment 18 Dobrica Pavlinusic 2013-03-21 09:57:57 UTC
This change will conflict with Bug 7973 so it should probably be re-based if 7973 gets into master first.

I think we should also provide example transformation module, because it's
non-obvious how to write transformation module from looking at code alone
(at least for me).

Good example of transformation module might be one which converts repeatable
values in ldap into multiple fields for easier import into koha since right
now koha just concatenates all values with spaces which is sub-optimal if you
want just one value from repeatable field.

I would love to write one, but I don't really know where to start.
Comment 19 Dobrica Pavlinusic 2013-03-21 10:10:23 UTC
I also don't like that existing LDAP configuration stops working with this patch applied (think about package upgrade) and only indication that something changed is message in error log: "deprecated ldap configuration, see documentation".

It might at least point to C4/LDAPAuthMethodTutorial.pod as source of that documentation, but ideal solution would be to support old configuration if possible.
Comment 20 Dobrica Pavlinusic 2013-03-21 14:53:17 UTC Comment hidden (obsolete)
Comment 21 Julian Maurice 2013-03-22 15:05:01 UTC Comment hidden (obsolete)
Comment 22 Julian Maurice 2013-03-22 15:15:59 UTC
With this patch and Dobrica's patch applied, I'm able to get into the login process: ldap binding is successful and if user is not existing in koha database, it is created.
Note: to really be able to login I had to modify C4::LDAPTransform::RepeatableValues to fill values for categorycode and branchcode since user creation fails if they are empty.
Comment 23 Dobrica Pavlinusic 2013-03-26 15:18:47 UTC
We should include all required fields (categorycode and branchcode) in example plugin so people don't have to look at this bug to find that out :-)

OOH, now that I have had enough sleep, is there really a good reason to introduce completely different configuration for LDAP server? Adding <branch>, <transform> and few other tags into existing configuration would probably result in simpler code and easier upgrade path.
Comment 24 Julian Maurice 2013-06-10 07:44:26 UTC Comment hidden (obsolete)
Comment 25 Julian Maurice 2013-06-10 07:44:40 UTC Comment hidden (obsolete)
Comment 26 Julian Maurice 2013-06-10 07:45:04 UTC Comment hidden (obsolete)
Comment 27 Julian Maurice 2013-06-10 07:45:23 UTC Comment hidden (obsolete)
Comment 28 Julian Maurice 2013-06-10 07:45:32 UTC Comment hidden (obsolete)
Comment 29 Julian Maurice 2013-06-10 07:49:40 UTC
(In reply to comment #23)
> OOH, now that I have had enough sleep, is there really a good reason to
> introduce completely different configuration for LDAP server? Adding
> <branch>, <transform> and few other tags into existing configuration would
> probably result in simpler code and easier upgrade path.

If you are refering to the use of xml attributes instead of xml tags, I think you can use the one you prefer, as:
  <ldapserver update="1" />
and
  <ldapserver>
    <update>1</update>
  </ldapserver>

will both result in a hash:
{
  update => "1"
}
Comment 30 I'm just a bot 2013-09-18 11:09:12 UTC
Applying: Bug 8993: LDAP authentication improvements
Using index info to reconstruct a base tree...
M	C4/Auth_with_ldap.pm
Falling back to patching base and 3-way merge...
Auto-merging C4/Auth_with_ldap.pm
CONFLICT (content): Merge conflict in C4/Auth_with_ldap.pm
Patch failed at 0001 Bug 8993: LDAP authentication improvements
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 31 Julian Maurice 2013-09-23 09:15:09 UTC Comment hidden (obsolete)
Comment 32 Julian Maurice 2013-09-23 09:15:15 UTC Comment hidden (obsolete)
Comment 33 Julian Maurice 2013-09-23 09:15:22 UTC Comment hidden (obsolete)
Comment 34 Julian Maurice 2013-09-23 09:15:28 UTC Comment hidden (obsolete)
Comment 35 Julian Maurice 2013-09-23 09:15:34 UTC Comment hidden (obsolete)
Comment 36 Julian Maurice 2013-09-23 09:15:53 UTC
All patches rebased on master
Comment 37 I'm just a bot 2013-09-23 22:04:00 UTC
Patch applied cleanly, go forth and signoff
Comment 38 I'm just a bot 2013-10-24 19:28:27 UTC
Applying: Bug 8993: LDAP authentication improvements
Using index info to reconstruct a base tree...
M	C4/Auth_with_ldap.pm
Falling back to patching base and 3-way merge...
Auto-merging C4/Auth_with_ldap.pm
CONFLICT (content): Merge conflict in C4/Auth_with_ldap.pm
Patch failed at 0001 Bug 8993: LDAP authentication improvements
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 39 Martin Renvoize 2013-10-30 12:01:30 UTC
Any chance of rebasing this again Julian.. I'm hoping to test it.
Comment 40 Julian Maurice 2013-10-30 12:22:58 UTC Comment hidden (obsolete)
Comment 41 Julian Maurice 2013-10-30 12:23:08 UTC Comment hidden (obsolete)
Comment 42 Julian Maurice 2013-10-30 12:23:21 UTC Comment hidden (obsolete)
Comment 43 Julian Maurice 2013-10-30 12:23:30 UTC Comment hidden (obsolete)
Comment 44 Julian Maurice 2013-10-30 12:23:40 UTC Comment hidden (obsolete)
Comment 45 Julian Maurice 2013-10-30 12:24:26 UTC
I rebased all patches but not tested them. Hope it will works :)
Comment 46 Martin Renvoize 2013-10-30 13:14:09 UTC
Great fast response Julian.. unfortunately I still can't get it to apply :(

Falling back to patching base and 3-way merge...
Auto-merging C4/Auth_with_ldap.pm
CONFLICT (content): Merge conflict in C4/Auth_with_ldap.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 8993: LDAP authentication improvements

Looking at the merge markers, it looks like the main routine 'checkpw_ldap' is the issue, but it's such an extensive re-write that I don't want to screw it up by trying to merge it myself.
Comment 47 Julian Maurice 2013-10-30 13:50:11 UTC
Hmm I forgot to fetch the git remote before rebasing...
Rebased patches are coming.
Comment 48 Julian Maurice 2013-10-30 13:50:49 UTC Comment hidden (obsolete)
Comment 49 Julian Maurice 2013-10-30 13:50:59 UTC Comment hidden (obsolete)
Comment 50 Julian Maurice 2013-10-30 13:51:11 UTC Comment hidden (obsolete)
Comment 51 Julian Maurice 2013-10-30 13:51:21 UTC Comment hidden (obsolete)
Comment 52 Julian Maurice 2013-10-30 13:51:31 UTC Comment hidden (obsolete)
Comment 53 Martin Renvoize 2013-10-30 13:59:06 UTC
Thanks Julian,

Applies beautifully... now for some testing :)
Comment 54 Mark Tompsett 2015-04-19 06:26:38 UTC
Some of this may also be obsolete. :(
Comment 55 Julian Maurice 2015-04-21 14:49:43 UTC Comment hidden (obsolete)
Comment 56 Julian Maurice 2015-04-21 14:49:53 UTC Comment hidden (obsolete)
Comment 57 Julian Maurice 2015-04-21 14:53:18 UTC
Patches rebased and cleaned.
All indentation-only stuff was removed from patch to ease reading it (and make it easier to apply in the future).
Comment 58 Julian Maurice 2015-04-21 15:01:58 UTC
Created attachment 38273 [details] [review]
Bug 8993: LDAP authentication improvements

1) allow multiple branch authentication
2) allow to use a Perl module to transform ldap entry into a Koha
borrower

These improvements needs koha-conf.xml to be modified. However, the old
authentication mechanism is still there, and if you do not change your
configuration file, it must continue to work exactly the same way.

There's a POD file describing new features C4/LDAPAuthMethodTutorial.pod
Comment 59 Julian Maurice 2015-04-21 15:02:03 UTC
Created attachment 38274 [details] [review]
[DO NOT PUSH] Bug 8993: Sample LDAP transformation module
Comment 60 Rui 2015-09-24 17:15:27 UTC
Apparently it is not possible to specify in the LDAP XML configuration something such as:

<base>OU=subunit,OU=unit,OU=supraunit,DC=dc1,DC=dc0;OU=othersubunit,OU=unit,OU=supraunit,DC=dc1,DC=dc0</base>

Only one search context is allowed at a given time, i.e.,

<base>OU=subunit,OU=unit,OU=supraunit,DC=dc1,DC=dc0</base> 

As far as I understand (sorry, very new to Koha and perl) this patch will solve the problem?

(I have one solution working already, with very minor changes to Auth_with_ldap.pm, based on this idea: 

my @base      = split(';',$ldap->{base})        or die ldapserver_error('base');

but would like to see a solution upstream)
Comment 61 Julian Maurice 2015-09-25 07:14:25 UTC
(In reply to Rui from comment #60)
> Apparently it is not possible to specify in the LDAP XML configuration
> something such as:
> 
> <base>OU=subunit,OU=unit,OU=supraunit,DC=dc1,DC=dc0;OU=othersubunit,OU=unit,
> OU=supraunit,DC=dc1,DC=dc0</base>
> 

It is possible with this patch, try something like this:

    <ldapserver uri="ldap://directory.example.com"
          manager="uid=koha,ou=people,dc=example,dc=com"
          password="MYS3CRET"
          authmethod="search_dn"
          replicate="1" update="1">
      <branch dn="ou=people,dc=site1,dc=example,dc=com" />
      <branch dn="ou=people,dc=site2,dc=example,dc=com" />
      <branch dn="ou=people,dc=site4,dc=example,dc=com" />
    </ldapserver>

This example comes from C4/LDAPAuthMethodTutorial.pod
Comment 62 Magnus Enger 2016-02-06 19:59:04 UTC
Will this patch make it possible to configure LDAPS? I'm an LDAP noob, but working with a customer I have managed to get LDAPS working, by changing line 109 of C4/Auth_with_ldap.pm +109 from this:

  my $db = Net::LDAP->new(\@hosts);

to this:

  my $db = Net::LDAP->new(\@hosts, version => 3, scheme => 'ldaps', port => 123, capath => '/usr/ssl/certs');

As far as I can tell, this patch does not touch that line, but I might be missing something... 

If this patch does not make LDAPS configurable, and if noone objects loudly, I'll do a patch on another bug that makes it possible to set version, scheme, port and capath in koha-conf.xml
Comment 63 Blou 2016-06-15 19:03:17 UTC
A few notes:
- update_local: you remove "my $password" while it's used below
- no doc on authmethod
- not sure about standard regarding xml attributes vs sub elements.
- Should explain what happens with old elements (auth_by_bind, timeout, etc...)
- debug_msg "ldap $_:", $msg->$_ for qw/ error code /; fails if $msg is undef
- function cnx returns an hash in case of error, then checks for undef to exit.  It failed, does not exit, then crash further trying to use the $cnx.

I really need this improvement, but lot of work needs to be done before QA gets on it.
Comment 64 Blou 2016-06-15 21:11:15 UTC
It works like I need it to.  So this feature gets my full support.
Now, to get it through QA, the points above (and many others) will need to be addressed.

If you need someone to sign it off, I can work with you while I have the damned LDAP knowledge in my mind AND and environment allowing me to test it.  Wait too long, and that's dead.

I like the flexibility it offers.  I could do all the complex mapping I wanted in my module.  But I had to explore and put a lot of "warn" in the code to get there.  Better doc would be helpful.

Btw, annoying that    

<mapping>
   <userid         is="name"></userid>
</mapping>

is still needed in koha-conf.xml.  The last mapping, but without it I couldn't get through.
Comment 65 Katrin Fischer 2017-05-12 13:36:26 UTC
The last 2 comments from Blou read like FQA - setting status. Julian, can you please comment?
Comment 66 Katrin Fischer 2018-10-28 17:11:56 UTC
> If you need someone to sign it off, I can work with you while I have the
> damned LDAP knowledge in my mind AND and environment allowing me to test it.
> Wait too long, and that's dead.

I assume that time window has closed now :(
Comment 67 Magnus Enger 2020-05-15 11:28:52 UTC
The code here is 5+ years old now. Should we just close the bug?
Comment 68 Blou 2020-05-15 11:37:31 UTC
we still used the code within the last year as-is.  Not saying the code should be in the master as-is, but the feature is useful to solve certain complicated LDAP environments.  

It's not like the school's library can dictate its IT department how to manage their systems, so customized mapping and multi access-points are a plus...
Comment 69 Julian Maurice 2020-05-15 12:28:42 UTC
I'm not willing to continue on this bug, so I don't mind closing it. I'm removing myself from the assignee field.
If still useful, it should probably be rewritten with plugins in mind.
Comment 70 Katrin Fischer 2020-05-16 09:56:19 UTC
Supporting multiple LDAP just came up here recently in a discussion about features that would be useful - maybe we should leave this open - even if the code is not ready for integration, it's still an enhancement request. We could reset it to 'NEW' maybe?
Comment 71 David Cook 2022-08-22 06:27:30 UTC
(In reply to Katrin Fischer from comment #70)
> Supporting multiple LDAP just came up here recently in a discussion about
> features that would be useful - maybe we should leave this open - even if
> the code is not ready for integration, it's still an enhancement request. We
> could reset it to 'NEW' maybe?

Hopefully bug 31378 could pave a path to making it easier to add multiple LDAP providers. 

I don't think I have any Koha clients using LDAP but it's not particularly complicated. With LDAP you get back a directory record, with OIDC you get a list of claims, with SAML you get a list of attributes. They're all amenable to the same type of mapping mechanism.