Bug 12026 - Shibboleth auto-provisioning - Create
Summary: Shibboleth auto-provisioning - Create
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 8446 13114 13569
Blocks: 12027 18507
  Show dependency treegraph
 
Reported: 2014-04-02 12:47 UTC by Martin Renvoize
Modified: 2017-05-21 21:52 UTC (History)
17 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
This feature allows it to add new patron records from Shibboleth into Koha ('provision'). It is possible to map Sbibboleth's attributes with Koha fields, the configuration is done in koha-conf.xml. Syncing existing Koha users with Shibboleth is not implemented yet.


Attachments
BUG12026: Adds auto-provisioning to shibboleth (2.54 KB, patch)
2014-04-02 15:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG12026: Adds auto-provisioning to shibboleth (2.64 KB, patch)
2014-04-03 08:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG12026: Adds auto-provisioning to shibboleth (2.83 KB, patch)
2014-05-21 14:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
[SIGNED OFF] BUG12026: Adds auto-provisioning to shibboleth (2.90 KB, patch)
2014-07-30 20:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026, Follow up: Added Documentation (2.82 KB, patch)
2014-07-30 20:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026, Follow up: Added Documentation (2.92 KB, patch)
2014-07-31 06:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
[SIGNED OFF] BUG12026: Adds auto-provisioning to shibboleth (3.06 KB, patch)
2014-08-21 15:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026, Follow up: Added Documentation (1.69 KB, patch)
2014-08-21 15:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
[SIGNED OFF] BUG12026: Adds auto-provisioning to shibboleth (3.15 KB, patch)
2015-01-27 11:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026, Follow up: Added Documentation (1.73 KB, patch)
2015-01-27 11:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG12026: Adds auto-provisioning to shibboleth (2.99 KB, patch)
2016-10-12 09:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026, Follow up: Added Documentation (1.97 KB, patch)
2016-10-12 09:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026: Followup Rebasing (2.31 KB, patch)
2016-10-12 10:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026 Fix small errors (1.24 KB, patch)
2016-11-15 10:43 UTC, Mirko Tietgen
Details | Diff | Splinter Review
BUG12026: Adds auto-provisioning to shibboleth (3.03 KB, patch)
2016-11-16 16:12 UTC, Mirko Tietgen
Details | Diff | Splinter Review
BUG 12026, Follow up: Added Documentation (2.03 KB, patch)
2016-11-16 16:13 UTC, Mirko Tietgen
Details | Diff | Splinter Review
BUG 12026: Followup Rebasing (2.36 KB, patch)
2016-11-16 16:13 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 12026 Fix small errors (1.24 KB, patch)
2016-11-16 16:13 UTC, Mirko Tietgen
Details | Diff | Splinter Review
BUG 12026: Adds auto-provisioning to shibboleth (3.09 KB, patch)
2017-03-20 11:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026, Follow up: Added Documentation (2.09 KB, patch)
2017-03-20 11:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026: Followup Rebasing (2.43 KB, patch)
2017-03-20 11:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026 Fix small errors (1.31 KB, patch)
2017-03-20 11:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: Adds auto-provisioning to shibboleth (3.13 KB, patch)
2017-03-21 11:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: [Follow up] Added Documentation (2.11 KB, patch)
2017-03-21 11:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: [Followup] Rebasing (2.47 KB, patch)
2017-03-21 11:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: Fix small errors (1.35 KB, patch)
2017-03-21 11:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: [QA Follow-up] POD typo and tiny change in AddMember_Opac (3.57 KB, patch)
2017-03-21 11:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
BUG 12026: Adds auto-provisioning to shibboleth (3.09 KB, patch)
2017-03-22 09:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026, Follow up: Added Documentation (2.09 KB, patch)
2017-03-22 09:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12026: Followup Rebasing (2.43 KB, patch)
2017-03-22 09:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026 Fix small errors (1.31 KB, patch)
2017-03-22 09:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: Adds auto-provisioning to shibboleth (3.09 KB, patch)
2017-03-22 13:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: [Follow up] Added Documentation (2.07 KB, patch)
2017-03-22 13:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: [Followup] Rebasing (2.43 KB, patch)
2017-03-22 13:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: Fix small errors (1.31 KB, patch)
2017-03-22 13:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: [QA Follow-up] POD typo and tiny change in AddMember_Opac (3.53 KB, patch)
2017-03-22 13:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: [Rebase Follow-up] - Fix Test (1.32 KB, patch)
2017-03-22 13:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: Adds auto-provisioning to shibboleth (3.19 KB, patch)
2017-03-23 11:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: [Follow up] Added Documentation (2.17 KB, patch)
2017-03-23 11:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: [Followup] Rebasing (2.52 KB, patch)
2017-03-23 11:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: Fix small errors (1.40 KB, patch)
2017-03-23 11:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: [QA Follow-up] POD typo and tiny change in AddMember_Opac (3.57 KB, patch)
2017-03-23 11:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: [Rebase Follow-up] - Fix Test (1.42 KB, patch)
2017-03-23 11:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 12026: [QA Follow-up] Fix a POD line (1.11 KB, patch)
2017-03-23 11:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Add Tests for autocreate (7.88 KB, patch)
2017-03-24 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Add Tests for autocreate (7.89 KB, patch)
2017-03-24 09:18 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: [QA Follow-up] Add Tests for autocreate (7.92 KB, patch)
2017-03-24 09:19 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12026: Adds auto-provisioning to shibboleth (3.21 KB, patch)
2017-03-29 14:32 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12026: [Follow up] Added Documentation (2.19 KB, patch)
2017-03-29 14:33 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12026: [Followup] Rebasing (2.55 KB, patch)
2017-03-29 14:33 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12026: Fix small errors (1.43 KB, patch)
2017-03-29 14:34 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12026: [QA Follow-up] POD typo and tiny change in AddMember_Opac (3.60 KB, patch)
2017-03-29 14:34 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12026: [Rebase Follow-up] - Fix Test (1.44 KB, patch)
2017-03-29 14:35 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12026: [QA Follow-up] Fix a POD line (1.14 KB, patch)
2017-03-29 14:35 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12026: [QA Follow-up] Add Tests for autocreate (7.51 KB, patch)
2017-03-29 14:36 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
BUG 12027: Added shibboleth authentication to the staff client (4.17 KB, patch)
2017-04-21 13:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
BUG 12027 [Followup]: Added missing documentation (1.33 KB, patch)
2017-04-21 13:20 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2014-04-02 12:47:45 UTC
Shibboleth authentication has the ability to send an arbitrary number of attributes to koha; These attributes can be used to dynamically create and update user records in koha.  Our shibboleth patches should support this.
Comment 1 Martin Renvoize 2014-04-02 15:56:08 UTC Comment hidden (obsolete)
Comment 2 Martin Renvoize 2014-04-02 15:57:56 UTC
Shamelessly grabbed the patches from Jesse's branch.. available here: https://github.com/pianohacker/koha/commits/shibboleth-v3.12.10

Re-based against master bug 8446, but kept original authorship as Jesse
Comment 3 Martin Renvoize 2014-04-03 08:15:34 UTC Comment hidden (obsolete)
Comment 4 Martin Renvoize 2014-05-21 14:07:23 UTC Comment hidden (obsolete)
Comment 5 Martin Renvoize 2014-07-30 20:20:17 UTC Comment hidden (obsolete)
Comment 6 Martin Renvoize 2014-07-30 20:20:25 UTC Comment hidden (obsolete)
Comment 7 Martin Renvoize 2014-07-31 06:37:46 UTC Comment hidden (obsolete)
Comment 8 Martin Renvoize 2014-08-21 15:06:14 UTC Comment hidden (obsolete)
Comment 9 Martin Renvoize 2014-08-21 15:06:20 UTC Comment hidden (obsolete)
Comment 10 Martin Renvoize 2015-01-27 11:42:22 UTC
Created attachment 35551 [details] [review]
[SIGNED OFF] BUG12026: Adds auto-provisioning to shibboleth

- Adds the option to map shibboleth attributes to koha user columns
  and allows mapping to auto create non-existant users

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2015-01-27 11:42:25 UTC
Created attachment 35552 [details] [review]
BUG 12026, Follow up: Added Documentation

BUG 12026, Follow up: Updated to work with match point selection

- The base bug that this patch enhances has added the ability to choose
  your match point in the configuration file. This patch adds code to
  take that change into account.
Comment 12 Martin Renvoize 2015-01-27 11:43:22 UTC
Rebased against master
Comment 13 Martin Renvoize 2015-01-27 12:04:58 UTC
Note to self: must add unit tests
Comment 14 Theodoros Theodoropoulos 2015-05-12 12:34:51 UTC
Very useful enhancement Martin! It is exactly what we need too :)

One small suggestion: An extra line that defines dbh is probably required in the beginning of checkpw_shib in case _autocreate has to be called
(ie something like: my $dbh = C4::Context->dbh; )

Other than that it's working like a charm for me!
Comment 15 Katrin Fischer 2015-05-12 12:39:19 UTC
Hi Theodoros, thx for testing this! I am setting to failed QA for now, as this will need a follow-up from Martin to get further. If you want, you could also sign off his patches as they are and add a follow-up for Martin to check and sign off on.
Comment 16 Martin Renvoize 2015-05-12 18:42:34 UTC
Thanks Theodoros, 

Think that must have crept in amongst the various rebases/iterations..

From a quick read of the code I actually thing the $dbh param can be removed from both the calling line and the sub unless I'm mistaken as it doesn't appear to be called upon at anywhere.

I'll do a quick test and a followup.
Comment 17 Theodoros Theodoropoulos 2015-06-04 05:32:25 UTC
Hello Martin,

If you're still looking into this bug, an additional useful enhancement would be to be able to handle custom patron attributes!

We have such a case where shibboleth returns a value that should go into a custom defined patron attribute and right now -although it's included it in the <mapping> tag- it is left blank, probably because ( $entry->{'is'} && $ENV{ $entry->{'is'} } ) is false

My knowledge of Koha code is very limited, but I think Auth_with_ldap.pm has such a handling, so you could take ideas from there.
Comment 18 Martin Renvoize 2016-10-12 09:58:14 UTC
Created attachment 56241 [details] [review]
BUG12026: Adds auto-provisioning to shibboleth

- Adds the option to map shibboleth attributes to koha user columns
  and allows mapping to auto create non-existant users
Comment 19 Martin Renvoize 2016-10-12 09:59:42 UTC
Created attachment 56242 [details] [review]
BUG 12026, Follow up: Added Documentation

BUG 12026, Follow up: Updated to work with match point selection

- The base bug that this patch enhances has added the ability to choose
  your match point in the configuration file. This patch adds code to
  take that change into account.
Comment 20 Martin Renvoize 2016-10-12 10:09:02 UTC
Created attachment 56243 [details] [review]
BUG 12026: Followup Rebasing
Comment 21 Mirko Tietgen 2016-11-15 10:43:36 UTC
Created attachment 57486 [details] [review]
Bug 12026 Fix small errors

When running tests I got two errors. Do these changes make sense?
Comment 22 Martin Renvoize 2016-11-16 11:06:52 UTC
They make perfect sense.. look like mostly rebaseing errors to me.. oops :(
Comment 23 Mirko Tietgen 2016-11-16 16:12:51 UTC
Created attachment 57563 [details] [review]
BUG12026: Adds auto-provisioning to shibboleth

- Adds the option to map shibboleth attributes to koha user columns
  and allows mapping to auto create non-existant users

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 24 Mirko Tietgen 2016-11-16 16:13:04 UTC
Created attachment 57564 [details] [review]
BUG 12026, Follow up: Added Documentation

BUG 12026, Follow up: Updated to work with match point selection

- The base bug that this patch enhances has added the ability to choose
  your match point in the configuration file. This patch adds code to
  take that change into account.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 25 Mirko Tietgen 2016-11-16 16:13:08 UTC
Created attachment 57565 [details] [review]
BUG 12026: Followup Rebasing

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Comment 26 Mirko Tietgen 2016-11-16 16:13:12 UTC
Created attachment 57566 [details] [review]
Bug 12026 Fix small errors

When running tests I got two errors. Do these changes make sense?
Comment 27 Marcel de Rooy 2017-01-06 08:20:52 UTC
Martin: Please answer Mirko's question on the last patch and signoff on that one. After that set status back to SO. Thanks !

Temporarily moving out of the SO queue.
Comment 28 Mirko Tietgen 2017-01-06 08:59:39 UTC
(In reply to Marcel de Rooy from comment #27)
> Martin: Please answer Mirko's question on the last patch and signoff on that
> one. After that set status back to SO. Thanks !
> 
> Temporarily moving out of the SO queue.

Martin replied on IRC saying they make sense.
Comment 29 Martin Renvoize 2017-03-20 11:11:04 UTC
Perfect Mirko, thanks for cleaning up the rebase.

Setting back to SO.
Comment 30 Martin Renvoize 2017-03-20 11:12:17 UTC
Just to clarify.. seems I've even been running Mirko's follow-up in production here on a couple of systems since they were done.. sorry for not having signed them off.. must have slipped through the net :(
Comment 31 Martin Renvoize 2017-03-20 11:15:48 UTC
Created attachment 61268 [details] [review]
BUG 12026: Adds auto-provisioning to shibboleth

- Adds the option to map shibboleth attributes to koha user columns
  and allows mapping to auto create non-existant users

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 32 Martin Renvoize 2017-03-20 11:15:55 UTC
Created attachment 61269 [details] [review]
BUG 12026, Follow up: Added Documentation

BUG 12026, Follow up: Updated to work with match point selection

- The base bug that this patch enhances has added the ability to choose
  your match point in the configuration file. This patch adds code to
  take that change into account.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 33 Martin Renvoize 2017-03-20 11:16:02 UTC
Created attachment 61270 [details] [review]
BUG 12026: Followup Rebasing

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 34 Martin Renvoize 2017-03-20 11:16:09 UTC
Created attachment 61271 [details] [review]
Bug 12026 Fix small errors

When running tests I got two errors. Do these changes make sense?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 35 Martin Renvoize 2017-03-20 11:17:00 UTC
Added signoff line to signify that I've checked the rebase and followup added by Mirko :)
Comment 36 Marcel de Rooy 2017-03-21 09:36:33 UTC
QA: Looking here now.
Comment 37 Marcel de Rooy 2017-03-21 11:14:05 UTC
Created attachment 61363 [details] [review]
Bug 12026: Adds auto-provisioning to shibboleth

- Adds the option to map shibboleth attributes to koha user columns
  and allows mapping to auto create non-existant users

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 38 Marcel de Rooy 2017-03-21 11:14:10 UTC
Created attachment 61364 [details] [review]
Bug 12026: [Follow up] Added Documentation

Updated to work with match point selection

- The base bug that this patch enhances has added the ability to choose
  your match point in the configuration file. This patch adds code to
  take that change into account.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Marcel de Rooy 2017-03-21 11:14:14 UTC
Created attachment 61365 [details] [review]
Bug 12026: [Followup] Rebasing

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 40 Marcel de Rooy 2017-03-21 11:14:19 UTC
Created attachment 61366 [details] [review]
Bug 12026: Fix small errors

When running tests I got two errors. Do these changes make sense?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 41 Marcel de Rooy 2017-03-21 11:14:24 UTC
Created attachment 61367 [details] [review]
Bug 12026: [QA Follow-up] POD typo and tiny change in AddMember_Opac

[1] Correct xml error in POD
Copy-pasting this example from Auth_with_shibboleth.pm into koha-conf.xml
did not work. We need a closing tag ;)
[2] If AddMember_Opac calls AddMember_Auto, there is no need to call
fixup_cardnumber. Adding trivial test for AddMember_Auto in Members.t.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 42 Marcel de Rooy 2017-03-21 11:16:40 UTC
After the first patch, test t/Auth_with_shibboleth.t fails.
Without it, it passes with me.
Please check.
Comment 43 Martin Renvoize 2017-03-22 09:46:39 UTC
Created attachment 61435 [details] [review]
BUG 12026: Adds auto-provisioning to shibboleth

- Adds the option to map shibboleth attributes to koha user columns
  and allows mapping to auto create non-existant users

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 44 Martin Renvoize 2017-03-22 09:47:03 UTC
Created attachment 61436 [details] [review]
BUG 12026, Follow up: Added Documentation

BUG 12026, Follow up: Updated to work with match point selection

- The base bug that this patch enhances has added the ability to choose
  your match point in the configuration file. This patch adds code to
  take that change into account.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 45 Martin Renvoize 2017-03-22 09:47:21 UTC
Created attachment 61437 [details] [review]
BUG 12026: Followup Rebasing

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 46 Martin Renvoize 2017-03-22 09:47:30 UTC
Created attachment 61438 [details] [review]
Bug 12026 Fix small errors

When running tests I got two errors. Do these changes make sense?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 47 Martin Renvoize 2017-03-22 09:54:03 UTC
I'm investigating those test failures today.. I'm totally baffled as to why ldap appears to be getting loaded by the tests after these patches, but not before.
Comment 48 Marcel de Rooy 2017-03-22 12:00:27 UTC
You changed the order of the patches.
And you obsoleted my corrections of the commit messages.
Could you restore them?
Comment 49 Marcel de Rooy 2017-03-22 12:32:56 UTC
Output of the test starts with:
1..11
not ok 1 - use C4::Auth_with_shibboleth;
#   Failed test 'use C4::Auth_with_shibboleth;'
#   at t/Auth_with_shibboleth.t line 100.
#     Tried to use 'C4::Auth_with_shibboleth'.
#     Error:  No ldapserver "hostname" defined in KOHA_CONF: /etc/koha/sites/master/koha-conf.xml at /usr/share/koha/masterclone/C4/Auth_with_ldap.pm line 59, <DATA> line 751.
Comment 50 Jonathan Druart 2017-03-22 13:42:58 UTC
-use_ok('C4::Auth_with_shibboleth');
+use C4::Auth_with_shibboleth;
+require_ok('C4::Auth_with_shibboleth');

This will fix the tests.
Do not ask me why and prefer to signoff on bug 17600 to avoid, maybe, such craziness...

See also t/db_dependent/Circulation.t, commit 3660c451a36db128c143b6388d0d1f34babff4ac for a similar issue.
Comment 51 Martin Renvoize 2017-03-22 13:58:23 UTC
Created attachment 61444 [details] [review]
Bug 12026: Adds auto-provisioning to shibboleth

- Adds the option to map shibboleth attributes to koha user columns
  and allows mapping to auto create non-existant users

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 52 Martin Renvoize 2017-03-22 13:58:31 UTC
Created attachment 61445 [details] [review]
Bug 12026: [Follow up] Added Documentation

Updated to work with match point selection

- The base bug that this patch enhances has added the ability to choose
  your match point in the configuration file. This patch adds code to
  take that change into account.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 53 Martin Renvoize 2017-03-22 13:58:38 UTC
Created attachment 61446 [details] [review]
Bug 12026: [Followup] Rebasing

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 54 Martin Renvoize 2017-03-22 13:58:46 UTC
Created attachment 61447 [details] [review]
Bug 12026: Fix small errors

When running tests I got two errors. Do these changes make sense?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 55 Martin Renvoize 2017-03-22 13:58:55 UTC
Created attachment 61448 [details] [review]
Bug 12026: [QA Follow-up] POD typo and tiny change in AddMember_Opac

[1] Correct xml error in POD
Copy-pasting this example from Auth_with_shibboleth.pm into koha-conf.xml
did not work. We need a closing tag ;)
[2] If AddMember_Opac calls AddMember_Auto, there is no need to call
fixup_cardnumber. Adding trivial test for AddMember_Auto in Members.t.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 56 Martin Renvoize 2017-03-22 13:59:04 UTC
Created attachment 61449 [details] [review]
Bug 12026: [Rebase Follow-up] - Fix Test
Comment 57 Martin Renvoize 2017-03-22 14:02:54 UTC
I've added the fix for the test as a follow-up and it also includes a tiny fix to Mirko's rebased code which I had overlooked.

Not sure if we need Mirko to retest for signoff or whether we can go back to QA ?
Comment 58 Marcel de Rooy 2017-03-23 10:46:35 UTC
QA: Looking here again
Comment 59 Marcel de Rooy 2017-03-23 11:16:10 UTC
Created attachment 61521 [details] [review]
Bug 12026: Adds auto-provisioning to shibboleth

- Adds the option to map shibboleth attributes to koha user columns
  and allows mapping to auto create non-existant users

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 60 Marcel de Rooy 2017-03-23 11:16:15 UTC
Created attachment 61522 [details] [review]
Bug 12026: [Follow up] Added Documentation

Updated to work with match point selection

- The base bug that this patch enhances has added the ability to choose
  your match point in the configuration file. This patch adds code to
  take that change into account.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 61 Marcel de Rooy 2017-03-23 11:16:20 UTC
Created attachment 61523 [details] [review]
Bug 12026: [Followup] Rebasing

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 62 Marcel de Rooy 2017-03-23 11:16:25 UTC
Created attachment 61524 [details] [review]
Bug 12026: Fix small errors

When running tests I got two errors. Do these changes make sense?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 63 Marcel de Rooy 2017-03-23 11:16:30 UTC
Created attachment 61525 [details] [review]
Bug 12026: [QA Follow-up] POD typo and tiny change in AddMember_Opac

[1] Correct xml error in POD
Copy-pasting this example from Auth_with_shibboleth.pm into koha-conf.xml
did not work. We need a closing tag ;)
[2] If AddMember_Opac calls AddMember_Auto, there is no need to call
fixup_cardnumber. Adding trivial test for AddMember_Auto in Members.t.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 64 Marcel de Rooy 2017-03-23 11:16:35 UTC
Created attachment 61526 [details] [review]
Bug 12026: [Rebase Follow-up] - Fix Test

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 65 Marcel de Rooy 2017-03-23 11:16:41 UTC
Created attachment 61527 [details] [review]
Bug 12026: [QA Follow-up] Fix a POD line

Still refers to a removed parameter of _autocreate.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 66 Marcel de Rooy 2017-03-23 11:17:51 UTC
QA Comment:
Code looks good to me.
This smaller change has mostly been code reviewed by me. I tested some constructs on itself, but I do not have a shibboleth server.
Since most QAers do not and it has been tested by Martin and Mirko, I take the liberty to pass QA on it now.
But not without a few remarks :)

Koha::Database->new()->schema()->resultset('Borrower')
      ->find( { $config->{matchpoint} => $match } );
Strictly speaking, you should use find with primary keys/unique constraints.
It might fallback to search here depending on your matchpoint (userid has a unique constraint).

=head2 _autocreate
  my ( $retval, $retcard, $retuserid ) = _autocreate( $config, $match );
Given a database handle [...]
=> Where is that database handle? It got removed after the second patch..
Fixed in a followup.

checkpw_shib
The change to this routine should normally be accompanied by changes in the tests.
Perhaps you should add autocreate => 0 explicitly to mockedConfig in this test.
I wonder if testing autocreate => 1 should be done better in a db_dependent test.
Since this patch goes such a long way, I do not want to block it now. But please add a small test on a new report.

Passed QA
Comment 67 Martin Renvoize 2017-03-23 13:12:20 UTC
Thanks Marcel, I'll certainly take a look at adding the requisite tests.. as you say the original work goes back so far that the tests were just not thought of at the time and not added in any of my follow-ups :(.

As for the call to find.. I agree, I'll add a further patch for that along with the tests (And include in those tests a case for a double matching matchpoint)

Glad to see this groundwork set passed :)
Comment 68 Martin Renvoize 2017-03-24 08:55:11 UTC
Created attachment 61596 [details] [review]
Add Tests for autocreate
Comment 69 Martin Renvoize 2017-03-24 08:55:59 UTC
In the name of doing it properly.. I've added the tests here :)
Comment 70 Martin Renvoize 2017-03-24 08:56:48 UTC
I'll do the switch from find to search single in another bug as it affects actual function.
Comment 71 Martin Renvoize 2017-03-24 09:18:12 UTC
Created attachment 61598 [details] [review]
Add Tests for autocreate
Comment 72 Martin Renvoize 2017-03-24 09:19:18 UTC
Created attachment 61599 [details] [review]
Bug 12026: [QA Follow-up] Add Tests for autocreate
Comment 73 Olli-Antti Kivilahti 2017-03-29 14:32:42 UTC
Created attachment 61675 [details] [review]
Bug 12026: Adds auto-provisioning to shibboleth

- Adds the option to map shibboleth attributes to koha user columns
  and allows mapping to auto create non-existant users

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 74 Olli-Antti Kivilahti 2017-03-29 14:33:03 UTC
Created attachment 61676 [details] [review]
Bug 12026: [Follow up] Added Documentation

Updated to work with match point selection

- The base bug that this patch enhances has added the ability to choose
  your match point in the configuration file. This patch adds code to
  take that change into account.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 75 Olli-Antti Kivilahti 2017-03-29 14:33:24 UTC
Created attachment 61677 [details] [review]
Bug 12026: [Followup] Rebasing

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 76 Olli-Antti Kivilahti 2017-03-29 14:34:33 UTC
Created attachment 61678 [details] [review]
Bug 12026: Fix small errors

When running tests I got two errors. Do these changes make sense?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 77 Olli-Antti Kivilahti 2017-03-29 14:34:58 UTC
Created attachment 61679 [details] [review]
Bug 12026: [QA Follow-up] POD typo and tiny change in AddMember_Opac

[1] Correct xml error in POD
Copy-pasting this example from Auth_with_shibboleth.pm into koha-conf.xml
did not work. We need a closing tag ;)
[2] If AddMember_Opac calls AddMember_Auto, there is no need to call
fixup_cardnumber. Adding trivial test for AddMember_Auto in Members.t.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 78 Olli-Antti Kivilahti 2017-03-29 14:35:22 UTC
Created attachment 61680 [details] [review]
Bug 12026: [Rebase Follow-up] - Fix Test

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 79 Olli-Antti Kivilahti 2017-03-29 14:35:50 UTC
Created attachment 61683 [details] [review]
Bug 12026: [QA Follow-up] Fix a POD line

Still refers to a removed parameter of _autocreate.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 80 Olli-Antti Kivilahti 2017-03-29 14:36:19 UTC
Created attachment 61684 [details] [review]
Bug 12026: [QA Follow-up] Add Tests for autocreate

https://bugs.koha-community.org/show_bug.cgi?id=12026
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 81 Olli-Antti Kivilahti 2017-03-29 14:36:55 UTC
Fixes failing test in t/Auth_with_shibboleth.t

Thanks for the fix!
Comment 82 Kyle M Hall 2017-03-31 15:09:05 UTC
Pushed to master for 17.05, thanks Jesse, Martin, Mirko, Marcel!
Comment 83 Katrin Fischer 2017-04-02 17:48:31 UTC
This won't get ported back to 16.11.x as it is an enhancement.
Comment 84 Katrin Fischer 2017-04-12 06:20:15 UTC
Is there documentation about this new feature available somewhere? Is it possible to choose if you want to add a new user and update existing users like it is with LDAP? Do the attributes have to match exactly to what Koha expects (branchcode, categorycode specifically) or is there a way to do a mapping?
Comment 85 Zeno Tajoli 2017-04-12 06:48:15 UTC
Hi Katrin,

the setup info are inside C4/Auth_with_shibboleth.pm in POD format:


Map shibboleth attributes to koha fields, and configure authentication match point in koha-conf.xml.

<shibboleth>
    <matchpoint>userid</matchpoint> <!-- koha borrower field to match upon -->
    <mapping>
      <userid is="eduPersonID"></userid> <!-- koha borrower field to shibboleth attribute mapping -->
    </mapping>
</shibboleth>
 
Note: The minimum you need here is a <matchpoint> block, containing a valid column name from the koha borrowers table, and a <mapping> block containing a relation between the chosen matchpoint and the shibboleth attribute name.
Comment 86 Katrin Fischer 2017-04-12 07:29:54 UTC
Thx Zeno - I looked at the patches, but didn't spot it!
Comment 87 Katrin Fischer 2017-04-12 07:32:20 UTC
Actually, after looking at the POD, I am still not sure what is the answer to my questions.
Comment 88 Zeno Tajoli 2017-04-12 09:47:41 UTC
Hi Katrin,

in fact to answer your question you need to read also the code.
I do some answers:

>Is there documentation about this new feature available somewhere?
The documentation is inside C4/Auth_with_shibboleth.pm in POD format.
To see it:
perldoc C4/Auth_with_shibboleth.pm

>Is it possible to choose if you want to add a new user and update existing users like it is with LDAP?

No. The system only adds a new user, it does not update an existing user. And it adds a new user only when, inside login process, it finds that there isn't an existing user with userid = the shibboleth attribure defined with this XML in koha-conf.xml:
<matchpoint>userid</matchpoint>
<mapping>
   <userid is="uid"></userid>
</mapping>

>Do the attributes have to match exactly to what Koha expects (branchcode, categorycode specifically) or is there a way to do a mapping?

You can mapping using koha-conf.xml. This is an example:

<matchpoint>userid</matchpoint>
<mapping>
   <userid is="uid"></userid>
   <surname is="sn"></surname>
   <categorycode is="tip"></categorycode>
   <branchcode is="cod"></branchcode>
   <email is="uid"></email>
   <emailpro is="mail"></emailpro>
</mapping>
The <matchpoint> ... </matchpoint> and the connected attribute are clearly mandatory.
Also attributes for categorycode and branchcode are mandatory, I think.
Ohters are optional.

Bye
Comment 89 Martin Renvoize 2017-04-12 11:39:03 UTC
You're totally right Katrin, I should expand the documentation a bit here.

This patch was only ever intended to 'add new' user record (i.e to 'provision') and not to 'sync'.

To be honest, I'd love to also submit a 'sync' patch and include an option to only sync certain fields but I'd likely need a bit of sponsorship to get that through.  I'm happy to code it up, but it's a nightmare to get these patches through signoff and qa ;)
Comment 90 Martin Renvoize 2017-04-21 13:20:33 UTC
Created attachment 62521 [details] [review]
BUG 12027: Added shibboleth authentication to the staff client

- This patch adds shibboleth authentication to the staff client.
- Depending upon how your url structure works, you may or may not need a
  second native shibboleth service provider profile configured for this
  to work.

http://bugs.koha-community.org/show_bug.cgi?id=12026
Comment 91 Martin Renvoize 2017-04-21 13:20:38 UTC
Created attachment 62522 [details] [review]
BUG 12027 [Followup]: Added missing documentation